[PATCH] D26508: [ELF] - Better diagnostic for relative relocation to an absolute value error.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 11 02:12:26 PST 2016


grimar added inline comments.


================
Comment at: ELF/InputFiles.cpp:471
           ->body();
-    return elf::Symtab<ELFT>::X->addRegular(Name, *Sym, Sec)->body();
+    return elf::Symtab<ELFT>::X->addRegular(Name, *Sym, Sec, this)->body();
   }
----------------
evgeny777 wrote:
> Why do you need to pass 'this' pointer if you can use Sec->getFile()
Because Sec can be nullptr, in case of absolute symbol.


================
Comment at: ELF/Relocations.cpp:345
       return true;
-    error("relocation " + getRelName(Type) +
+    error(getFilename(Body.File) + "relocation " + getRelName(Type) +
           " cannot refer to absolute symbol " + Body.getName());
----------------
evgeny777 wrote:
> I suggest printing getLocation(Rel.Section, Rel.Offset) as it seems to be more useful than location of absolute symbol. Also please add ": " after location.
Done, printing both, thanks for hint.


================
Comment at: ELF/SymbolTable.h:63
                      uintX_t Value, uintX_t Size, uint8_t Binding,
-                     InputSectionBase<ELFT> *Section);
+                     InputSectionBase<ELFT> *Section, InputFile *File);
   Symbol *addRegular(StringRef Name, const Elf_Sym &Sym,
----------------
pcc wrote:
> Why is it necessary to add this parameter, can't you access the file via the section?
Section is null for absolute symbols.


https://reviews.llvm.org/D26508





More information about the llvm-commits mailing list