Skip to content
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

frontend fixes #251

Merged
merged 1 commit into from
Oct 2, 2024
Merged

frontend fixes #251

merged 1 commit into from
Oct 2, 2024

Conversation

ailrst
Copy link
Contributor

@ailrst ailrst commented Sep 23, 2024

  • handle unsupported opcodes from Unsupported opcode fallback gtirb-semantics#22
  • disambiguate functions with local binding (c static) which have the same name but different addresses in the boogie output
  • handle relf files with empty relocation table
  • some missing relf enum values

Comment on lines +224 to +230
val dupProcNames = (ctx.program.procedures.groupBy(_.name).filter((n,p) => p.size > 1)).toList.flatMap(_._2)

var dupCounter = 0
for (p <- dupProcNames) {
dupCounter += 1
p.name = p.name + "$" + p.address.map(_.toString).getOrElse(dupCounter.toString)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good to have but may potentially cause problems elsewhere because there are some things (like indirect call resolution) that rely on the names of procedures in a somewhat fragile way. That is something that I want to fix in the indirect call resolution though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes its not correct to rely on names it should go through symbol table address -> basil procedure address, we should probably be clear about what the correct unique procedure identifier is in the IR though.

@ailrst ailrst merged commit 5aa5c6f into main Oct 2, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants