[PATCH] D100651: [AIX] Support of Big archive (read)

Tue Sep 14 05:43:21 PDT 2021

EGuesnet added inline comments.

================
Comment at: llvm/lib/Object/Archive.cpp:1013
+    return child_end();
+  const char *Loc = Data.getBufferStart() + strlen(Magic) + sizeof(Archive::ArFixLenHdrType);
+  Child C(this, Loc, &Err);
----------------
Esme wrote:
> EGuesnet wrote:
> > Esme wrote:
> > > It's not correct to calculate the offset of the first archive member by `Data.getBufferStart() + strlen(Magic) + sizeof(Archive::ArFixLenHdrType);`, please use the value of  `ArFixLenHdr->FirstArOffset`.
> > > You can easily reproduce the bug if you test a archive file which contains a object file member like the comment I added before:
> > > ```
> > > $ xlc 1.c -o 1.o
> > > $ ar -v -q 1.a 1.o
> > > $ llvm-ar tv 1.a
> > > llvm-ar: error: unable to load '1.a': truncated or malformed archive (characters in size field in archive header are not all decimal numbers: '\000\000\000\000\000\000\000\000\000\0005892' for archive member header at offset 128)
> > > ``` 
> > > The correct offset for this case should be 138 instead of 128.
> > First, LLVM is no more a priority of our team, so I cannot spend time to this PR.
> > Second, I am not able to reproduce. Please give me content of 1.c, and content of 1.a.
> > ```
> > // small.c
> > int add_two (int a) {
> >         return a + 2;
> > }
> > 
> > $ xlc -v
> > C for AIX Compiler, Version 5
> > 
> > OK, really old, but we don't care for archive.
> > 
> > $ xlc small.c  -o small.o -c
> > $ ar -v -q small.a small.o
> > 
> > $ xxd small.a
> > 
> > 00000000: 3c62 6967 6166 3e0a 3130 3730 2020 2020  <bigaf>.1070
> > 00000010: 2020 2020 2020 2020 2020 2020 3132 3332              1232
> > 00000020: 2020 2020 2020 2020 2020 2020 2020 2020
> > 00000030: 3020 2020 2020 2020 2020 2020 2020 2020  0
> > 00000040: 2020 2020 3132 3820 2020 2020 2020 2020      128
> > 00000050: 2020 2020 2020 2020 3132 3820 2020 2020          128
> > 00000060: 2020 2020 2020 2020 2020 2020 3020 2020              0
> > 00000070: 2020 2020 2020 2020 2020 2020 2020 2020
> > // End of Fixed-Length Header: size is 128
> > 00000080: 3832 3020 2020 2020 2020 2020 2020 2020  820
> > 00000090: 2020 2020 3130 3730 2020 2020 2020 2020      1070
> > 000000a0: 2020 2020 2020 2020 3020 2020 2020 2020          0
> > 000000b0: 2020 2020 2020 2020 2020 2020 3136 3331              1631
> > 000000c0: 3630 3435 3039 2020 3020 2020 2020 2020  604509  0
> > 000000d0: 2020 2020 3020 2020 2020 2020 2020 2020      0
> > 000000e0: 3634 3420 2020 2020 2020 2020 3720 2020  644         7
> > [...]
> > 
> > $ llvm-ar --version
> > LLVM version 13.0.0
> > 
> > $ llvm-ar t small.a
> > small.o # OK
> > ```
> > 
> > Third, according documentation https://www.ibm.com/docs/en/aix/7.2?topic=formats-ar-file-format-big , Fixed-Length Header must have a size of 128. 138 is not valide.
> Thanks for your reply!
> 1. If you are no longer following up on this patch, I'd be happy to continue your work. How do you think?
> 
> 2. In my previous comment, a binary member is added to the archive, while in your case, there is an object member (compiled with `-c` option) in archive.
> You can also reproduce the issue if you add a dynamic lib to an archive.
> ``` 
> $ cat 1.c
> int main() {
> return 1;
> }
> $ xlc 1.c -qmkshrobj -o libt.so
> $ ar -v -q 1.a libt.so
> $ xxd 1.a 
> 00000000: 3c62 6967 6166 3e0a 3134 3532 2020 2020  <bigaf>.1452    
> 00000010: 2020 2020 2020 2020 2020 2020 3136 3134              1614
> 00000020: 2020 2020 2020 2020 2020 2020 2020 2020                  
> 00000030: 3020 2020 2020 2020 2020 2020 2020 2020  0               
> 00000040: 2020 2020 3133 3420 2020 2020 2020 2020      134         
> 00000050: 2020 2020 2020 2020 3133 3420 2020 2020          134     
> 00000060: 2020 2020 2020 2020 2020 2020 3020 2020              0   
> 00000070: 2020 2020 2020 2020 2020 2020 2020 2020                  
> 00000080: 0000 0000 0000 3131 3935 2020 2020 2020  ......1195    
> // There are some padding between the Fixed-Length Header and the First member.  
> 00000090: 2020 2020 2020 2020 2020 3134 3532 2020            1452  
> 000000a0: 2020 2020 2020 2020 2020 2020 2020 3020                0 
> 000000b0: 2020 2020 2020 2020 2020 2020 2020 2020
> ```
> 
> 3. It's correct that `Fixed-Length Header must have a size of 128. 138 is not valide.` As above, there may be some padding between the Fixed-Length Header and the first member. So I think the value of  `ArFixLenHdr->FirstArOffset` is the exact offset to the first member.
Using ArFixLenHdr->FirstArOffset does not work.
```
truncated or malformed archive (characters in name length field in archive header are not all decimal numbers: '' for archive member header at offset 68)
```
I do not know where this "68" is from.

In my opinion, this PR is complex enough. It might be accepted with as few changes as possible. So, without freelist, undocumented features... In a second time, you can create a new PR, to extend support of Big Archive, with new tests.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100651/new/

https://reviews.llvm.org/D100651