-
Notifications
You must be signed in to change notification settings - Fork 150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add DWARF4 base type entry encodings to symtabAPI::typeScalar #1059
Conversation
bool is_string; | ||
|
||
// Detailed properties | ||
bool is_address; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about some kind of boolean for is_null, would that be useful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was looking at this, and I think we can add it here. DWARF treats 'void' separately from the base_type attributes handled here, but I think we can lump them together. I was going to chat with @sashanicolas first before adding it.
https://bottle.cs.wisc.edu/search?dyninst_branch=PR1059 No new regressions. I want to modify an existing test to explicitly check for the properties here (insofar as C will let us) before merging. |
@sashanicolas Sorry to press, but we need these changes before we can move forward with Smeagle. Could you take a look as soon as you get a chance? |
Thank you @sashanicolas ! +1 what @hainest said. Smeeeeagle needs to parse the preccccious (but he's really using dyninst under the hood 😆 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
https://bottle.cs.wisc.edu/search?dyninst_branch=PR1059&testsuite_branch=PR196 No new regressions. |
Woohoo! Will this be in a release soon (how does the release schedule work for Dyninst?) |
@vsoch We don't have a fixed release schedule, but we try to do at least two point releases per year. We do patch releases as necessary. We just did one of those a couple weeks ago, so it might be a while before we do another one (unless there's a pressing reason). For now, I would recommend updating Dyninst in the dev container to the current HEAD with these changes. |
Gotcha. Autamus (our current development base) only builds from releases so what I'll do is create the same container manually from dyninst master, and then we can use that (and update it) as long as we need. I have it building now, and I can add the Dockerfile and updates to the repo the next time I need to make changes! |
If anyone wants / needs the Dockefile, here you go! It installs dyninst via spack, and I've found this much easier than trying to get it to work locally.
|
These properties are needed for doing ABI analysis in Smeagle.
ping @vsoch