[PATCH] D81258: [yaml2obj] - Introduce a ~10 Mb limit of the output by default and a --nolimit option.
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 8 02:40:56 PDT 2020
grimar marked an inline comment as done.
grimar added inline comments.
================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:908-909
+ // --nolimit yaml2obj command line option.
+ raw_ostream &OS = *CBA.getRawOS(0x100000 * 8); // 8Mb.
+ uint64_t BeginOffset = CBA.tell();
if (Name == ".debug_str")
----------------
Higuoxing wrote:
> jhenderson wrote:
> > This feels like a bit of a hack to get this to work with the DWARFYAML interface. @Higuoxing, how straightforward do you think it would be to modify the DWARFYAML interface to take a `ContiguousBlobAccumulator`? I'm guessing not particularly, since we'd need to move it into a shared header etc, but thought I'd ask.
> >
> > @grimar, as an alternative, maybe you could create a new CBA here, with a size limit equal to the remaining size available in the original CBA. You'd then append its buffer to the original, once you'd checked the size limit hadn't been breached.
> I think it's not easy to change the interface. `MachOEmitter` doesn't use the `ContiguousBlobAccumulator`.
> since we'd need to move it into a shared header etc
Sharing `ContiguousBlobAccumulator` is a reasonable move I think. I thought about it when wrote this, as doing this should allow to reuse it for non-ELF emitters.
> as an alternative, maybe you could create a new CBA here, with a size limit equal to the remaining size available in the original CBA.
Ok, I'll try. But it is still not ideal. While we use `raw_ostream` in the `DWARFYAML` directly, it is possible to break everything. Imagine I create `ContiguousBlobAccumulator`. with a limit of 10 Mb here, but `DWARFYAML` tries to write 10 Gb.
We might face a OOM inside `DWARFYAML` logic before having a chance to check that the size limit is breached here.
So the only right way is changing the DWARFYAML interface I believe. I think for now we can go with your suggestion and leave `DWARFYAML` changes for a follow-up?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81258/new/
https://reviews.llvm.org/D81258
More information about the llvm-commits
mailing list