| | [ Return to Bugs & Features | SVN ⇄ GIT ]
STR #3420
| Application: | FLTK Library |
|---|
| Status: | 1 - Closed w/Resolution |
|---|
| Priority: | 4 - High, e.g. key functionality not working |
|---|
| Scope: | 3 - Applies to all machines and operating systems |
|---|
| Subsystem: | FLUID |
|---|
| Summary: | fluid can crash on certain use of static types |
|---|
| Version: | 1.4-current |
|---|
| Created By: | greg.ercolano |
|---|
| Assigned To: | greg.ercolano |
|---|
| Fix Version: | 1.4.0 (SVN: v12531) |
|---|
| Fix Commit: | 9c2b0d743f2c3ecd7f1475c0ccf2c9f6e5805436 |
|---|
| Update Notification: | |
|---|
Trouble Report Files:
No files
Trouble Report Comments:
| |
| #1 | greg.ercolano
18:16 Oct 27, 2017 |
| Making this STR on behalf of Vladimir Markelov
who wrote in fltk.coredev on 10/19/17:
"""
Looking through the latest sources (1.4 build 12490) I noticed the following block of code:
fluid/Fl_Function_Type.cxx Line: #278
if (rtype) {
if (!strcmp(rtype,"static")) {is_static = 1; rtype = 0;}
else if (!strncmp(rtype, "static ",7)) {is_static = 1; rtype += 7;}
if (!strcmp(rtype, "virtual")) {is_virtual = 1; rtype = 0;}
else if (!strncmp(rtype, "virtual ",8)) {is_virtual = 1; rtype += 8;}
}
In case of rtype equals "static" then it gets 0 value and then it may crash the app at the next 'if'. Should the code be with extra else:
if (rtype) {
if (!strcmp(rtype,"static")) {is_static = 1; rtype = 0;}
else if (!strncmp(rtype, "static ",7)) {is_static = 1; rtype += 7;}
else if (!strcmp(rtype, "virtual")) {is_virtual = 1; rtype = 0;}
else if (!strncmp(rtype, "virtual ",8)) {is_virtual = 1; rtype += 8;}
}
?
"""
This is true; the fact that this line sets rtype=0:
if (!strcmp(rtype,"static")) {is_static = 1; rtype = 0;}
..means that any other strcmp() on rtype will crash. | |
| |
| #2 | greg.ercolano
18:21 Oct 27, 2017 |
| My reply to Vladimir's suggestion on fltk.coredev:
"""
Interesting.. yes, that looks bad, since rtype is being set to 0.
If this is a bug, it's been in there a loooong time, those lines
haven't been touched since r650:
r650 | bill | 1999-08-16 22:33:12 -0700 (Mon, 16 Aug 1999) | 2 lines
You can make virtual destructors
BTW, perhaps another solution, to follow the pattern used
in that section of the code:
if (rtype) {
if (!strcmp(rtype,"static")) {is_static = 1; rtype = 0;}
else if (!strncmp(rtype, "static ",7)) {is_static = 1; rtype += 7;}
}
if (rtype) {
if (!strcmp(rtype, "virtual")) {is_virtual = 1; rtype = 0;}
else if (!strncmp(rtype, "virtual ",8)) {is_virtual = 1; rtype += 8;}
}
Your solution is probably fine too; I don't think 'static' would
ever be followed by 'virtual' (no such thing as a 'static virtual', I think?).
""" | |
| |
| #3 | greg.ercolano
11:02 Oct 28, 2017 |
| OK, to trigger this crash, you have to create a new function
and specify just "static". This would create in code:
static foo()
..which is not really legal C++, as there's no type specified (void,int,etc)
..but to trigger this bug that's what's needed, which is probably why no one's
encountered it. (The OP just noticed the problem in code)
The only situation where I think it's legal to declare a function without a type
is when declaring a class constructor or destructor. But I don't think there's
such a thing as a "static constructor" or "static destructor". So again, not
legal C++, and therefore unlikely to encounter.
Anyway, replication:
1) Create a new fluid project.. File -> New
2) New -> Function, then for "Return Type:" specify: static
(just the word "static", no spaces)
3) File -> Write Code (seg faults) | |
| |
| #4 | greg.ercolano
14:23 Oct 28, 2017 |
| Fixed in Subversion repository. | |
| |
| #5 | AlbrechtS
05:05 Jan 31, 2021 |
| I stumbled across this issue in branch-1.3 and backported the fix:
see branch-1.3: commit beb733e39e7935d0f7336515d1ae402897c847a9
... together with two other fixes in the same file.
FTR: the crash can really happen and the generated code would get a default return type 'void' or 'Fl_Window *' or whatever is appropriate
in the context, hence there's no illegal code generated. Verified with
a simple fluid file as suggested above. | |
[ Return to Bugs & Features ]
|
| |
