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

[Rui Ueyama via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/InputFiles.cpp:807-814
   elf::Symtab<ELFT>::X->addRegular(StartName, STV_DEFAULT, STT_OBJECT, 0, 0,
-                                   STB_GLOBAL, Section);
+                                   STB_GLOBAL, Section, nullptr);
   elf::Symtab<ELFT>::X->addRegular(EndName, STV_DEFAULT, STT_OBJECT,
-                                   Data.size(), 0, STB_GLOBAL, Section);
+                                   Data.size(), 0, STB_GLOBAL, Section,
+                                   nullptr);
   elf::Symtab<ELFT>::X->addRegular(SizeName, STV_DEFAULT, STT_OBJECT,
+                                   Data.size(), 0, STB_GLOBAL, nullptr,
----------------
Why don't you pass `this` instead of `nullptr`?


================
Comment at: ELF/Relocations.cpp:309-310
 static bool isStaticLinkTimeConstant(RelExpr E, uint32_t Type,
-                                     const SymbolBody &Body) {
+                                     const SymbolBody &Body,
+                                     InputSectionBase<ELFT> &S,
+                                     typename ELFT::uint RelOff) {
----------------
I don't think you need to add a new parameter because DefinedRegular has Section member.


================
Comment at: ELF/Relocations.cpp:349
+          " cannot refer to absolute symbol '" + Body.getName() +
+          "', symbol defined here " + getFilename(Body.File));
     return true;
----------------
Please change the message "cannot refer to absolute symbol '" + Body.getName() + "' defined in "


================
Comment at: ELF/Relocations.cpp:637
     uint32_t Type = RI.getType(Config->Mips64EL);
+    uintX_t Off = RI.r_offset;
 
----------------
Revert this change because this is not related.


================
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,
----------------
grimar wrote:
> 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.
Actually, I hope we represent an absolute symbol in a different way than a null pointer so that we can eliminate the parameter. You don't need to do that in this patch though.


https://reviews.llvm.org/D26508