[PATCH] D87179: Fix debug_abbrev emitter to only assign table id once

Sat Sep 12 23:55:04 PDT 2020

aadsm added a comment.

@Higuoxing I didn't realize the DWARF section was completely new until I read your GSOC project. Thanks a lot for this work otherwise it would have been impossible for me to create the tests I needed for my other diff!

================
Comment at: llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml:683

-# RUN: yaml2obj --docnum=3 %s | obj2yaml | FileCheck %s --check-prefix=MULTI-TABLES
+# RUN: yaml2obj --docnum=3 %s | obj2yaml | FileCheck %s --check-prefix=MULTI-TABLES --dump-input=always

----------------
jhenderson wrote:
> Remove `--dump-input` from the FileCheck command.
> 
> I actually have rarely used the option myself - you can achieve the goal of getting FileCheck to dump the input by having no check prefixes that match the prefix being looked for, which is often a good starting point.
Not sure if I understand what you mean with "having no check prefixes that match the prefix being looked for," but I'll remove this! I added for debugging to see where the mismatch was but forgot to remove it.

================
Comment at: llvm/tools/obj2yaml/dwarf2yaml.cpp:27-28
     for (auto AbbrvDeclSet : *AbbrevSetPtr) {
+      if (AbbrvDeclSet.second.begin() == AbbrvDeclSet.second.end())
+        continue;
       Y.DebugAbbrev.emplace_back();
----------------
jhenderson wrote:
> Higuoxing wrote:
> > Do we really need this check here? I think it will prevent us from dumping an empty abbrev table.
> > 
> > ```
> > debug_abbrev:
> >     - ID: 0
> >       Table:
> >         - Code:           1
> >           Tag:            DW_TAG_compile_unit
> >           Children:       DW_CHILDREN_yes
> >           Attributes:
> >             - Attribute:  DW_AT_producer
> >               Form:       DW_FORM_strp
> >     - ID: 1   ## This table will not be dumped.
> > ```
> Could we get a test case for this situation, please? Either separately or as part of this patch, I don't mind.
That is true. I didn't realize there should always be an empty table at the end of the debug_abbrev. I'll update then.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87179