Closed
Bug 429031
Opened 16 years ago
Closed 16 years ago
Expose more information about precision and signedness via Dehydra Javascript interface
Categories
(Developer Infrastructure :: Source Code Analysis, defect)
Developer Infrastructure
Source Code Analysis
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: bk2, Assigned: vladimir.sukhoy)
References
Details
Attachments
(1 file, 3 obsolete files)
11.37 KB,
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.13) Gecko/20080311 Firefox/2.0.0.13 Build Identifier: GCC provides several important pieces of information which are not exposed for scripting via Javascript in Dehydra: 1) TYPE_MIN_VALUE 2) TYPE_MAX_VALUE 3) TYPE_PRECISION 4) TYPE_UNSIGNED These fields are vital for static analysis of C++ code for security purposes. Exposing these fields to Javascript would allow some important questions to be answered in script. Commonly asked questions during a security code review: -> Where are all the instances of assigning an unsigned value to a signed type? For example: unsigned int ui = 0xFFFFFFFF; int i = ui; -> Where are all the instances of truncation (data-loss) via assignment? For example: int i = 99999; short s = i; Reproducible: Always Steps to Reproduce: 1. 2. 3.
Assignee | ||
Comment 1•16 years ago
|
||
Ok, let's not touch align and machine modes for now and only do this + unit tests.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment 2•16 years ago
|
||
Vlad, I assume you were trying to assign this to yourself?
Assignee: nobody → vladimir.sukhoy
Status: ASSIGNED → NEW
Assignee | ||
Comment 3•16 years ago
|
||
Yes, and messed up at that (urgh, actually this is the second time this precise thing happens with me).
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•16 years ago
|
||
Assignee | ||
Comment 5•16 years ago
|
||
Now with min and max.
Attachment #315692 -
Attachment is obsolete: true
Comment 6•16 years ago
|
||
(In reply to comment #3) > Yes, and messed up at that (urgh, actually this is the second time this precise > thing happens with me). Yeah, I was told you can't change the status to assigned and reassign the bug to a person in one step.
Comment 7•16 years ago
|
||
What's the status of this work? Ben are you still working on the analysis? Vlad, the patch looks good. The dehydra_ast part is something that needs to be done better(expr_as_string as it is done currently is a crappy approach). Your changes are an improvement of existing situation. I think I could be happy with the other changes too, but I need to think about it some more(and test/use cases).
Blocks: 423898
Assignee | ||
Comment 8•16 years ago
|
||
Nevermind the AST part (there's a bug there actually), I think it is worth doing in bug 429446 or something equivalent. Actually I was working on testcases with expr_as_string approach but they were failing because of bug 429362 so I filed bug 429446. Not sure if we want to do testcases with expr_as_string or just go ahead with better representation and do testcases there..
Assignee | ||
Comment 9•16 years ago
|
||
Folded dmandelin's patch from bug 429362 into this, got rid of gcc pretty printer until it is fixed and handled 64-bit ints and type suffixes there, added more constants of various types and added support for precision on pointer/reference, enumeral type and for signedness on enumeral types.
Attachment #315703 -
Attachment is obsolete: true
Assignee | ||
Comment 10•16 years ago
|
||
Noticed a minor glitch in the test which'd break js strict mode, otherwise the same.
Attachment #317330 -
Attachment is obsolete: true
Assignee | ||
Updated•16 years ago
|
Attachment #317336 -
Flags: review?(dmandelin)
Comment 11•16 years ago
|
||
I really like your testcases, so i was about to commit this, however I'm not sure i like {name:"unsigned char" unsigned:true} {unsigned:false, precision:8, name:"signed char"}. This seems redundant. Either the signed/unsigned part should be taken out of the name or it shouldn't be an extra property. I'm also unsure about unsigned:false, a) Dehydra just doesn't include properties when they are false b) seems kind of backwards, signed:true might work better? While at it how about prefixing boolean properties with is? Having said that, perhaps integers are a special enough case to cause rules a) and b) to be bent. Thoughts?
Assignee | ||
Comment 12•16 years ago
|
||
Comment on attachment 317336 [details] [diff] [review] patch v2 I think isSigned:true/isSigned undefined is a good idea. Getting rid of signedness within .name.. Can be done, but I dunno if it will help or not.
Attachment #317336 -
Flags: review?(dmandelin)
Comment 13•16 years ago
|
||
it'd have to be isUnsigned i think, since C types default to signed
Comment 14•16 years ago
|
||
Vlad, mind if I finish up the details myself?
Comment 15•16 years ago
|
||
pushed with is(Uns|S)igned mods
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Core → Firefox Build System
Updated•1 year ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•