[PATCH] D78927: [yaml2obj] - Introduce the "Offset" property for sections.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 15 03:02:41 PDT 2020


grimar marked an inline comment as done.
grimar added inline comments.


================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:48
-    return OS;
-  }
 
----------------
FTR, after commiting this patch I faced LLD test failture and fixed it:
https://github.com/llvm/llvm-project/commit/969c63a2ecfb536062ff2174645abe31e4036067
(it started to report "section [index 1] has a sh_offset (0x1000000000000000) + sh_size (0x4) that is greater than the file size (0x100000120)" instead of "section sh_addralign is too large" LLD's error)

The issue was related to "AddressAlign:    0x1000000000000000" line in the YAML:

```
  - Name:            .text
    Type:            SHT_PROGBITS
    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
    AddressAlign:    0x1000000000000000
    Content:         "00000000"
```


Previously, we had this piece of code which used `unsigned`, so `AddressAlign` was truncated.
As a result a little binary with `sh_addrallign==0x1000000000000000` was produced.
After this patch, this piece does not exist and currently for that test a 500mb binary with with `sh_addrallign==0x1000000000000000` is produced. I.e. it works a bit more correct, but
the binary is is still truncated.

It happens because of `ELFState<ELFT>::alignToOffset`, which has the following lines:

```
  CBA.getOS().write_zeros(AlignedOffset - CurrentOffset);
  return AlignedOffset;
```

`AlignedOffset`  and `CurrectOffset` are `uint64_t`, but `raw_ostream::write_zeros` takes `unsigned`:

```
/// indent - Insert 'NumSpaces' spaces.
raw_ostream &raw_ostream::indent(unsigned NumSpaces) {
  return write_padding<' '>(*this, NumSpaces);
}

/// write_zeros - Insert 'NumZeros' nulls.
raw_ostream &raw_ostream::write_zeros(unsigned NumZeros) {
  return write_padding<'\0'>(*this, NumZeros);
}
```

Probably we want to change it to take `uint64_t`.
I do not think there is a way to write a test for it though as the result output would be huge.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78927





More information about the llvm-commits mailing list