This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Linkerscript: add support for suffixes in numbers.
ClosedPublic

Authored by grimar on Sep 2 2016, 7:46 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 70165.Sep 2 2016, 7:46 AM
grimar retitled this revision from to [ELF] - Linkerscript: add support for suffixes in numbers..
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: llvm-commits, grimar, davide, evgeny777.
ruiu added inline comments.Sep 2 2016, 8:05 AM
ELF/LinkerScript.cpp
1235 ↗(On Diff #70165)

Something's not right with this function. You seems to be mixing two different concepts, base and multiplier in this function, which confused me while reading this code. You cannot use K/M suffixes for hex numbers, so hex numbers should be handled first.

The code should be something like this.

if (Tok.startswith_lower("0x"))
  return Tok.substr(2).getAsInteger(16, Result);
if (Tok.endswith_lower("H"))
  return Tok.drop_back().getAsInteger(16, Result);

int Suffix = 1;
if (Tok.endswith_lower("K")) {
  Suffix = 1024;
  Tok = Tok.drop_back();
} else if (Tok.endswith_lower("M")) {
  Suffix = 1024 * 1024;
  Tok = Tok.drop_back();
}
if (Tok.getAsInteger(10, Result))
  return false;
Result *= Suffix;
return true;
1236 ↗(On Diff #70165)

Delete this enum and use these numbers directly. They will never change.

grimar updated this revision to Diff 70170.Sep 2 2016, 8:54 AM
  • Addressed review comments.
ELF/LinkerScript.cpp
1235 ↗(On Diff #70165)

Done.

ruiu accepted this revision.Sep 2 2016, 9:04 AM
ruiu edited edge metadata.

LGTM with this change.

ELF/LinkerScript.cpp
1326–1331 ↗(On Diff #70170)

Please try to simplify when you flip the meaning of the return value.

// Tok is a literal number.
uint64_t V;
if (readInteger(Tok, V))
  return [=](uint64_t Dot) { return V; };

// Tok is a symbol name.
if (Tok != "." && !isValidCIdentifier(Tok))
  setError("malformed number: " + Tok);
return [=](uint64_t Dot) { return getSymbolValue(Tok, Dot); };
This revision is now accepted and ready to land.Sep 2 2016, 9:04 AM
This revision was automatically updated to reflect the committed changes.