DanielT / a2ltool

A tool to edit, merge and update a2l files
Apache License 2.0
46 stars 15 forks source link

Missing measurements for bitfields #27

Closed oleid closed 4 months ago

oleid commented 4 months ago

The latest commit (ddaab36d3ed7dca1b564d29a8f74939a73c160dc) seems to result in the a2l file lacking information regarding bitfields.

The a2l file I'm updating which was generated by CANape10 contains i.e. the following measurement:

    /begin MEASUREMENT ErrorLog.bit.SomeError1 ""
      UBYTE NO_COMPU_METHOD 0 0 0 1
      BIT_MASK 0x2
      ECU_ADDRESS 0x400216AB
      ECU_ADDRESS_EXTENSION 0x0
      FORMAT "%.15"
      /begin IF_DATA CANAPE_EXT
        100
        LINK_MAP "ErrorLog.bit.SomeError1" 0x400216AB 0x0 0 0x0 1 0x80 0x1
        DISPLAY 0 0 1
      /end IF_DATA
      SYMBOL_LINK "ErrorLog.bit.SomeError1" 0
    /end MEASUREMENT

up till now the following was generated when using --update

    /begin MEASUREMENT ErrorLog.bit.SomeError1""
      ULONG NO_COMPU_METHOD 0 0 0 1
      BIT_MASK 0x40000000
      ECU_ADDRESS 0x400216A8
      ECU_ADDRESS_EXTENSION 0x0
      FORMAT "%.15"
      /begin IF_DATA CANAPE_EXT
        100
        LINK_MAP "ErrorLog.bit.SomeError1" 0x400216A8 0x0 0 0x0 1 0x80 0x1E
        DISPLAY 0 0 1
      /end IF_DATA
      SYMBOL_LINK "ErrorLog.bit.SomeError1" 0
    /end MEASUREMENT

(aparrently we use the inverted bitfields setting in CANape, thus the mask is different). But now the measurements for bits are completely missing.

(*) This is by the C code, by the way

typedef union __ErrorLogFlags
{
    struct
    { 
        uint32_t ModulNo             : 10;
        uint32_t reserved            : 19;
        uint32_t SomeError2          : 1;                    // 0x00004
        uint32_t SomeError1          : 1;                    // 0x00002
        uint32_t SomeError0          : 1;                    // 0x00001
    } bit;
    uint32_t all;
} ErrorLog;
DanielT commented 4 months ago

Sadly, you've hit a bad point to test - The last commit enables INSTANCEs and TYPEDEF_*, but other code needs to be adapted to handle the changes in the type information. That's currently unfinished in my local working tree.

I made some improvements to the handling of bitfields before that change though, so you could try going back to ff2c6c30. Specifically, I found that different versions of the DWARF standard handle bitfields differently: In DWARF 2/3 the encoding is endian-dependent, whereas for DWARF 4/5 it uses a new tag and is independent of endianness.

oleid commented 4 months ago

Sadly, you've hit a bad point to test - The last commit enables INSTANCEs and TYPEDEF_*, but other code needs to be adapted to handle the changes in the type information. That's currently unfinished in my local working tree.

Yeah, the last commit produces the same output as my previous test with v1.6.0.

I made some improvements to the handling of bitfields before that change though, so you could try going back to ff2c6c3. Specifically, I found that different versions of the DWARF standard handle bitfields differently: In DWARF 2/3 the encoding is endian-dependent, whereas for DWARF 4/5 it uses a new tag and is independent of endianness.

The debugger tells me that my binaries (generated using Diab for 32 bit power architecture) indeed hit the big endian DWARF 2/3 branch. But I'm not really sure yet, if the bit location is correctly encoded in the dwarf debugging info or not. Or why the inverted bit field option is needed in CANape (maybe only to handle its own shortcomings?). In any case, that's a different issue.

EDIT:

For future reference. llvm-dwarfdump clearly says:

0x0000a3a7:     DW_TAG_member
                  DW_AT_name    ("SomeError1")
                  DW_AT_type    (0x000000000000b2dd "unsigned int")
                  DW_AT_byte_size       (4)
                  DW_AT_bit_size        (0x01)
                  DW_AT_bit_offset      (0x1e)
                  DW_AT_data_member_location    (DW_OP_plus_uconst 0x0)
                  DW_AT_accessibility   (DW_ACCESS_public)

So from what I can tell, a2ltool does the correct thing based on the given information.

DanielT commented 4 months ago

So, I have a hypothesis: What if Canape always converts the mask to little-endian? I'm sure Canape just transfers the whole element and then applies the mask locally, so it internally it needs the BIT_MASK in little-endian format. That would mean the mask of a big-endian target would be flipped, while the mask of a little-endian target would not be flipped.

oleid commented 4 months ago

So, I have a hypothesis: What if Canape always converts the mask to little-endian? I'm sure Canape just transfers the whole element and then applies the mask locally, so it internally it needs the BIT_MASK in little-endian format. That would mean the mask of a big-endian target would be flipped, while the mask of a little-endian target would not be flipped.

This is what I speculated as well (cf. PR #28 ). Big question is: how to nicely make a "CANape-mode" available? Would you accept some command line flag?

DanielT commented 4 months ago

I just created pull request #29 which is how I think it should probably work.

Essentially, the bit index from the dwarf 2/3 debug data is now always reversed, instead of only for little endian. Instead the dwarf 4/5 case has the endian dependent reversal, since input is encoded in an endian-independent form.

I don't think a command line parameter is reuqired here. From what I've seen, the Vector tools accept many different map file formats, some of which presumably don't encode the input endianness the way an elf file does. In that case you would need such a flag.

If you have time, please try out the "bit_mask_endian" / pr #29 branch and let me know if that fixes the issue for you.

oleid commented 4 months ago

I just created pull request #29 which is how I think it should probably work. [...] If you have time, please try out the "bit_mask_endian" / pr #29 branch and let me know if that fixes the issue for you.

Indeed, it creates the same output as #28. I think you can merge it.