[PATCH] D22625: [ELF] Support PROVIDE and PROVIDE_HIDDEN within SECTIONS {} block
Rui Ueyama via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 21 06:32:58 PDT 2016
ruiu added inline comments.
================
Comment at: ELF/LinkerScript.cpp:283
@@ -283,3 +282,3 @@
Dot = Val;
- } else {
+ else if (Cmd->Provision != SymbolProvision::Ignore) {
auto *D = cast<DefinedRegular<ELFT>>(Symtab<ELFT>::X->find(Cmd->Name));
----------------
So, is it equivalent to `Cmd->Provision == SymbolProvision::Create`, no?
================
Comment at: ELF/LinkerScript.cpp:799-807
@@ -778,5 +798,11 @@
std::vector<StringRef> ScriptParser::readSectionsCommandExpr() {
+ int Braces = 0;
std::vector<StringRef> Expr;
while (!Error) {
- StringRef Tok = next();
+ StringRef Tok = peek();
+ Braces += (Tok == "(");
+ Braces -= (Tok == ")");
+ if (Braces < 0)
+ break;
+ next();
if (Tok == ";")
----------------
grimar wrote:
> evgeny777 wrote:
> > grimar wrote:
> > > evgeny777 wrote:
> > > > ruiu wrote:
> > > > > Is this change related to the feature you are adding in this patch?
> > > > Yes. As I said in summary, this is intended to early terminate the expression reader in case we see extra ')' bracket. Otherwise parser will eat tokens till it finds semicolon
> > > This should be fixed in separate patch.
> > May be, but actually this parser improvement is only needed for PROVIDE(). I don't mind making separate review, but not sure if it's really needed. Rui, what's your opinion?
> Point here that this feature is useful itself and also should have a testcase which is unrelative to what this patch do. That not only make this patch a bit smaller, but also leaves this improvement in if patch be reverted, for example.
I don't think we even want it. If there's an error, it'll be found out, right?
Repository:
rL LLVM
https://reviews.llvm.org/D22625
More information about the llvm-commits
mailing list