[PATCH] D61532: implement of the parsing symbol table for xcoffobjfile and output as yaml format
Hubert Tong via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed May 15 09:39:21 PDT 2019
hubert.reinterpretcast added inline comments.
================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:100
+ Symb.p = reinterpret_cast<uintptr_t>(SymEntPtr);
return;
}
----------------
sfertile wrote:
> this is unneeded.
There's still a stray `return` statement here.
================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:443
+ // field
+ if (!checkSize(Data, EC, CurPtr, (StringTable.Size)))
+ return;
----------------
Why the extra parentheses around `StringTable.Size`?
================
Comment at: llvm/lib/ObjectYAML/XCOFFYAML.cpp:20
+#include <cstdint>
+#include <vector>
----------------
Both of the above includes are included directly by `XCOFFYAML.h` for which this is the implementation file.
================
Comment at: llvm/test/tools/obj2yaml/aix_xcoff.test:17
+# CHECK-NEXT: Section: N_DEBUG
+# CHECK-NEXT: Type: 0x0003
+# CHECK-NEXT: StorageClass: C_FILE
----------------
DiggerLin wrote:
> DiggerLin wrote:
> > sfertile wrote:
> > > Is the intent to always keep dumping the type as a numeric value? For this tool I think that's reasonable but I'd like to clarify.
> > for the N_ABS, N_DEBUG, N_UNDEF, which do not has section related to it, It not show section number here.
> Thanks for Hubert let me I make a wrong answer.
>
> 1. for the question of " Is the intent to always keep dumping the type as a numeric value? For this tool I think that's reasonable but I'd like to clarify."
>
> Answer:
> in the xcoff document ,
> n_type :
> Bits 0-3
> Symbol visibility. The SYM_V_MASK macro, with the value 0xF000, can be used to mask off bits in the n_type field that do not specify visibility. The following visibilities are defined:
> 0x1000 SYM_V_INTERNAL
> 0x2000 SYM_V_HIDDEN
> 0x3000 SYM_V_PROTECTED
> 0x4000 SYM_V_EXPORTED
>
> Bit 10
> Optionally set to 1 if the symbol is a function. Otherwise, it is set to 0.
>
> I can not use enum to represent the value. I have to display as a value.
>
> 2. For the question "This is a C_FILE entry where n_lang is C (0x00) and n_cputype is COM (0x03). That is, perhaps we should have inspected CFileLanguageIdAndTypeId?"
>
> Answer: I do not think yaml framework support showing different field name based on the value of the field.
>
> The field name of yaml output are defined in the function, Once it is defined , we can not change the field name based on the value.
>
> void MappingTraits<XCOFFYAML::Symbol>::mapping(IO &IO, XCOFFYAML::Symbol &S) {
> IO.mapRequired("Name", S.SymbolName);
> IO.mapRequired("Value", S.Value);
> IO.mapRequired("Section", S.SectionName);
> IO.mapRequired("Type", S.Type);
> IO.mapRequired("StorageClass", S.StorageClass);
> IO.mapRequired("NumberOfAuxEntries", S.NumberOfAuxEntries);
> }
>
What happens with something like:
```
llvm/lib/ObjectYAML/COFFYAML.cpp: IO.mapOptional("SymbolTableIndex", Rel.SymbolTableIndex);
```
?
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61532/new/
https://reviews.llvm.org/D61532
More information about the llvm-commits
mailing list