This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Fix incorrect logic in VersionScriptParser::parseVersion()
ClosedPublic

Authored by grimar on Jun 23 2016, 3:41 AM.

Details

Summary

Previously the next sample script would generate 2 entries in
Config->SymbolVersions with the same version name.

VERSION {
 global: c;
};

That happened because parseVersionSymbols() was called twice.
At first for "global:" and since there is no local tag, it was called again.
Patch fixes the issue, testcase update to demonstrate.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 61659.Jun 23 2016, 3:41 AM
grimar retitled this revision from to [ELF] - Fix incorrect logic in VersionScriptParser::parseVersion().
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar.
grimar updated this revision to Diff 61660.Jun 23 2016, 3:47 AM
  • Reduced testcase.
grimar updated this object.Jun 23 2016, 3:50 AM
ruiu added inline comments.Jun 23 2016, 10:44 PM
ELF/SymbolListFile.cpp
93 ↗(On Diff #61660)

Why don't you add

else if (peek() != "}")

here?

grimar added inline comments.Jun 24 2016, 12:31 AM
ELF/SymbolListFile.cpp
93 ↗(On Diff #61660)

That was one of the ways and first what I wanted to do. My way looks more clean for my eye, though I am fine with both variants.
I`ll update the patch according to your suggestion then.

grimar updated this revision to Diff 61762.Jun 24 2016, 1:15 AM
  • Addressed review comments.
ruiu accepted this revision.Jun 24 2016, 3:56 AM
ruiu edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jun 24 2016, 3:56 AM
This revision was automatically updated to reflect the committed changes.