[PATCH] D112465: [llvm-profgen] Avoid writing any data to CSNameTableSection for the empty CSNameTable
Lei Wang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 25 18:34:30 PDT 2021
wlei added inline comments.
================
Comment at: llvm/lib/ProfileData/SampleProfWriter.cpp:255-256
std::error_code SampleProfileWriterExtBinaryBase::writeCSNameTableSection() {
+ if (CSNameTable.empty())
+ return sampleprof_error::success;
+
----------------
wenlei wrote:
> `SampleProfileReaderExtBinaryBase::readCSNameTableSec` would still expect to read a zero here, and if we skip writing the zero, the read will get some garbage data? Or did I miss anything?
>
> Also this is inconsistent with how we write other sections - we don't omit zero size.
You see the header reader logic below, at the beginning of the loop, it will skip the zero size entry (` if (!Entry.Size)`). so it won't call the `readCSNameTableSec`. Also it has the sanity check after read(`if (Data != SecStart + SecSize)`)
```
for (auto &Entry : SecHdrTable) {
// Skip empty section.
if (!Entry.Size)
continue;
...
if (std::error_code EC = readOneSection(SecStart, SecSize, Entry))
return EC;
if (Data != SecStart + SecSize)
return sampleprof_error::malformed;
....
```
I feel like each section doesn't follow strong encoding convention. `NameTable` is encoded the size probably because it won't have a zero size(if the profile is not empty).
Like the ProfileSymbolList, it can be a zero section size:
```
if (ProfSymList && ProfSymList->size() > 0)
if (std::error_code EC = ProfSymList->write(*OutputStream))
return EC;
return sampleprof_error::success;
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112465/new/
https://reviews.llvm.org/D112465
More information about the llvm-commits
mailing list