This is an archive of the discontinued LLVM Phabricator instance.

Add dsymutil dwarf5 tests for darwin
ClosedPublic

Authored by rastogishubham on Jun 1 2023, 1:51 PM.

Details

Summary

There are two simple tests to make sure dsymutil does the right thing when emitting dwarf5 for darwin platforms.

The first test, dwarf5-darwin.test checks to make sure that dsymutil generates the correct headers and sections for a darwin binary with dwarf5 in it

The second test, dwarf5-dwarf4-comb.test checks to make sure that dsymutil generates the correct headers and sections for a darwin binary with which was linked with one object file with dwarf4 and the other with dwarf 5 in it.

We can see that in this case, dsymutil will generate dwarf4 and dwarf5, it doesn't do an "upgrade" to dwarf5 for everything. This is consistent with how it behaves when linking something with dwarf4 and dwarf2 as well

Diff Detail

Event Timeline

rastogishubham created this revision.Jun 1 2023, 1:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2023, 1:51 PM
rastogishubham requested review of this revision.Jun 1 2023, 1:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2023, 1:51 PM
avl added a subscriber: avl.Jun 2 2023, 3:55 AM
avl added inline comments.
llvm/test/tools/dsymutil/ARM/dwarf5-darwin.test
1 ↗(On Diff #527596)

probably, it should have

# REQUIRES: system-darwin

?

Thanks for adding more tests!

I have two high level questions:

  1. Are the standard_opcode_lengths CHECKs relevant for what we are trying to test here?
  2. Is the traditional approach for dsymutil tests to have a YAML object file? I see that the same directory has tests that assemble an object file from .ll files (by calling llc) and also tests that assembly an object file from assembly (.s) by calling llvm-mc. Both seem shorter / easier to follow, but I have never looked much into those tests.
bulbazord added inline comments.Jun 2 2023, 3:48 PM
llvm/test/tools/dsymutil/ARM/dwarf5-darwin.test
1 ↗(On Diff #527596)

My understanding is that shouldn't be needed. You can run most dsymutil tests on non-darwin systems.

avl added inline comments.Jun 3 2023, 2:12 AM
llvm/test/tools/dsymutil/ARM/dwarf5-darwin.test
1 ↗(On Diff #527596)

The test is named as if it has something Darwin specific. If it is not then, probably, it would be better to name it without mentioning Darwin. Also, it would be good if the test would describe itself in the very beginning. Something like:

## This test checks that simple MachO binary contains consistent DWARFv5 info.

or

## This test checks that dsymutil generates the correct headers and sections for a MachO binary linked with one object file containing DWARFv4 and the other containing DWARFv5.
avl added a comment.Jun 4 2023, 2:18 AM
  1. Is the traditional approach for dsymutil tests to have a YAML object file? I see that the same directory has tests that assemble an object file from .ll files (by calling llc) and also tests that assembly an object file from assembly (.s) by calling llvm-mc. Both seem shorter / easier to follow, but I have never looked much into those tests.

using YAML object file seems OK if debug info is presented in an explicit way:

DWARF:
  debug_abbrev:
    - Table:
      - Tag:      DW_TAG_compile_unit
        Children: DW_CHILDREN_yes
        Attributes:
          - Attribute: DW_AT_producer
            Form:      DW_FORM_string
          - Attribute: DW_AT_language
            Form:      DW_FORM_data2
          - Attribute: DW_AT_name
            Form:      DW_FORM_string
  debug_info:
    - Version: 4
      Entries:
        - AbbrCode: 1
          Values:
            - CStr: by_hand
            - Value:  0x04
            - CStr: CU1
aprantl requested changes to this revision.Jun 5 2023, 4:47 PM
aprantl added inline comments.
llvm/test/tools/dsymutil/ARM/dwarf5-darwin.test
1 ↗(On Diff #527596)

+1 on commenting what the test does.

As for naming the test, we could call it -macho instead of -darwin since it's about the container format more than the platform?

As @bulbazord said, the REQUIRES line should not be needed.

3 ↗(On Diff #527596)

Either I'm confused by this test, or the test is doing something stange, but you can probably get rid of the .yaml file entirely.

dsymutil takes an executable mach-o binary (or a dylib) as input. So the input file's extension should probably not be .o.
But we also shouldn't need to create one, because dsymutil can take a debug map (~ what dsymutil -s would print) in yaml as input. This is why most tests just use -y %p/dummy-debug-map.map, which is a debug map that looks for a files named 1.o 2.o 3.o. So if you name your files accordingly (look at what the other tests are doing), you should be able to do the same.

llvm/test/tools/dsymutil/ARM/dwarf5-dwarf4-comb.test
1 ↗(On Diff #527596)

I assume -comb stands for combined? Maybe just spell it out?

This revision now requires changes to proceed.Jun 5 2023, 4:47 PM

Updated test to no longer use the yaml files, but use the dummy-debug-map instead

rastogishubham marked 3 inline comments as done.Jun 6 2023, 10:06 AM
rastogishubham added inline comments.
llvm/test/tools/dsymutil/ARM/dwarf5-darwin.test
3 ↗(On Diff #527596)

It was meant to be .out or something, not .o, that is my mistake, but I will rewrite the test to use the debug-map

rastogishubham marked an inline comment as done.Jun 6 2023, 10:08 AM

Thanks for adding more tests!

I have two high level questions:

  1. Are the standard_opcode_lengths CHECKs relevant for what we are trying to test here?
  2. Is the traditional approach for dsymutil tests to have a YAML object file? I see that the same directory has tests that assemble an object file from .ll files (by calling llc) and also tests that assembly an object file from assembly (.s) by calling llvm-mc. Both seem shorter / easier to follow, but I have never looked much into those tests.

@fdeazeve I doubt the standard opcode lengths vary too much, I just wanted to check to make sure that the whole line table header looks okay

aprantl added inline comments.Jun 6 2023, 11:12 AM
llvm/test/tools/dsymutil/ARM/dummy-debug-map-amr64.map
1

Is there a type in the file name!?

llvm/test/tools/dsymutil/ARM/dwarf5-dwarf4-combination-macho.test
2

typo: DWARF

3

We tend to spell this DWARF v5 in most places.

31

typo: 2.o

llvm/test/tools/dsymutil/ARM/dwarf5-macho.test
2

checks to ensure that -> checks that

3

dSYM

9

why is this needed?

rastogishubham marked 5 inline comments as done.

Fixed typos in comments

aprantl added inline comments.Jun 7 2023, 2:22 PM
llvm/test/tools/dsymutil/ARM/dwarf5-macho.test
2

DWARF v5 (makes for easier grepping)

aprantl added inline comments.Jun 7 2023, 2:25 PM
llvm/test/tools/dsymutil/ARM/dwarf5-dwarf4-combination-macho.test
31

Just FYI: -gdwarf4 is a driver option with the same effect.

44

It would be good to CHECK for a DW_FORM inside of a DW_AT_location in both CUs to make sure they point to the respective variant of the location list section.

45

Same thing for the ranges section.

Changed tests to make sure that the debug_rangelist and debug_loclists contents are in the the DWARF5 CU and the debug_loc and debug_ranges contents are in the dwarf4 cu

aprantl accepted this revision.Jun 14 2023, 9:45 AM
This revision is now accepted and ready to land.Jun 14 2023, 9:45 AM
This revision was landed with ongoing or failed builds.Jun 15 2023, 9:53 AM
This revision was automatically updated to reflect the committed changes.