remove alignment for debug data #1434

abroooo · 2025-03-13T14:49:10Z

This PR fixes the issue of values not showing for methods of an inherited functionblock while debugging. The reason for this wrong behavior is an alignment issue where structs are not aligned in the way the debugger expects them to be. The runtime behavior is not affected by this issue. As discussed with @ghaith we'll remove alignment for debug data for now. This remedies the issue.

TODO:

fix tests
fix snapshot tests
discuss strings
cleanup

mhasel · 2025-03-17T15:09:35Z

I have tried to debug a quick example and it seems we cannot remove the alignment in the debuginfo after all. I'm getting quite a few garbage values in the debugger.

TYPE struct_ : STRUCT
        inner: inner;
        inner_arr: ARRAY[0..2] OF inner;
        s : STRING := 'Hello';
        b : BOOL := TRUE;
        r : REAL := 3.1415;
        arr: ARRAY[0..2] OF STRING := ['aa', 'bb', 'cc'];
        i : INT := 42;
    END_STRUCT
END_TYPE

TYPE inner : STRUCT
        s : STRING := 'Hello';
        b : BOOL := TRUE;
        r : REAL := 3.1415;
        arr: ARRAY[0..2] OF STRING := ['aaaa', 'bbbb', 'cccc'];
        i : INT := 42;
    END_STRUCT
END_TYPE

FUNCTION main 
VAR
    st: struct_;
    s : STRING;
    b : BOOL;
    arr: ARRAY[0..2] OF STRING;
    i : INT;
END_VAR
    s := st.s;
    s := st.inner.s;
    b := st.b;
    b := st.inner.b;
    arr := st.arr;
    arr := st.inner.arr;
    i := st.i;
    i := st.inner.i;
    // arr := ['', '', ''];
    arr[0] := st.arr[0];
    arr[1] := st.inner.arr[1];
    arr[2] := st.inner.arr[2];

    printf('s: %s$N', REF(st.s));
    printf('s: %s$N', REF(st.inner.s));

    printf('arr at 0: %s$N', REF(st.arr[0]));
    printf('arr at 1: %s$N', REF(st.arr[1]));
    printf('arr at 2: %s$N', REF(st.arr[2]));

    printf('inner arr at 0: %s$N', REF(st.inner.arr[0]));
    printf('inner arr at 1: %s$N', REF(st.inner.arr[1]));
    printf('inner arr at 2: %s$N', REF(st.inner.arr[2]));

    printf('inner struct arr at 0, inner string arr at 0: %s$N', REF(st.inner_arr[0].arr[0]));
END_FUNCTION

plc <file> <printf.pli> -g -Onone --linker clang -o out
gdb out

After stopping on a breakpoint in main, using info locals in gdb shows the following:

Breakpoint 1, main () at target/demo.st:42
42          s := st.s;
(gdb) info locals
st = {inner = {s = "Hello", '\000' <repeats 75 times>, b = true, r = 2.63775365e-30, arr = {"I@aaaa", '\000' <repeats 74 times>, "\000\000bbbb", '\000' <repeats 74 times>, "\000\000cccc", '\000' <repeats 74 times>}, i = 0}, inner_arr = {{s = "\000*", '\000' <repeats 78 times>,
      b = false, r = 0, arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}, i = 0}, {s = '\000' <repeats 80 times>, b = false, r = 0, arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}, i = 0}, {
      s = '\000' <repeats 80 times>, b = false, r = 0, arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}, i = 0}}, s = '\000' <repeats 20 times>, "Hello", '\000' <repeats 55 times>, b = false, r = 0, arr = {
    '\000' <repeats 15 times>, "\001\000\000V\016I@aa", '\000' <repeats 56 times>, '\000' <repeats 22 times>, "bb", '\000' <repeats 56 times>, '\000' <repeats 22 times>, "cc", '\000' <repeats 56 times>}, i = 0}
s = '\000' <repeats 80 times>
b = false
arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}
i = 0

After trying this on master, I've noticed that the output is not correct either (also tried before the latest commit in regards to END_XXX and initializer debug changes).
Here's the gdb output on master:

Breakpoint 1, main () at target/demo.st:46
46          arr := st.arr;
(gdb) info locals
st = {inner = {s = "Hello", '\000' <repeats 75 times>, b = true, r = 3.1415, arr = {"aaaa", '\000' <repeats 76 times>, "bbbb", '\000' <repeats 76 times>, "cccc", '\000' <repeats 76 times>}, i = 42}, inner_arr = {{s = '\000' <repeats 80 times>, b = false, r = 0, arr = {
        '\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}, i = 0}, {s = '\000' <repeats 80 times>, b = false, r = 0, arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}, i = 0}, {s = '\000' <repeats 80 times>,
      b = false, r = 0, arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}, i = 0}}, s = "\000\000\000\000\000\000Hello", '\000' <repeats 69 times>, b = false, r = 0, arr = {"\000\001\000\000V\016I@aa", '\000' <repeats 70 times>,
    "\000\000\000\000\000\000\000\000bb", '\000' <repeats 70 times>, "\000\000\000\000\000\000\000\000cc", '\000' <repeats 70 times>}, i = 0}
s = "Hello", '\000' <repeats 75 times>
b = true
arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}
i = 0

Notice that here the first floating point value is still correct, but soon after this statement is no longer true - we are back to being confronted with garbage.
Yet another thing I've noticed is that deeply nested structs, in this case the array of the inner struct within the struct do not seem to be initialized correctly, since the printf output is correct for all the values in the above example except for the last one, which simply prints nothing (i.e. the string starts nul-terminated). So maybe the garbage values in the debugger are a false-positive due to the values not being initialized, we're going to have to look into this further.

mhasel · 2025-03-17T14:14:20Z

src/codegen/debug.rs

@@ -310,7 +309,7 @@ impl<'ink> DebugBuilder<'ink> {
                        file,
                        location.get_line_plus_one() as u32,
                        size.bits().into(),
-                        alignment.bits(),
+                        0, // no alignment for now


The wording of this comment is a bit confusing - can we either remove it or give a reason as to why there is no alignment? Someone reading this 6 months down the line/someone not aware of this change might not appreciate the mystery 😄

mhasel · 2025-03-17T14:14:35Z

src/codegen/debug.rs

        //Create a struct type
        let struct_type = self.debug_info.create_struct_type(
            file.as_debug_info_scope(),
            name,
            file,
            location.get_line_plus_one() as u32,
            running_offset.bits().into(),
-            struct_dt.get_alignment(index).bits(),
+            0, // no alignment for now


mhasel · 2025-03-17T14:14:42Z

src/codegen/debug.rs

@@ -458,7 +455,7 @@ impl<'ink> DebugBuilder<'ink> {
            file,
            location.get_line_plus_one() as u32,
            file.as_debug_info_scope(),
-            inner_dt.get_type_information().get_alignment(index).bits(),
+            0, // no alignment for now


mhasel · 2025-03-17T14:17:59Z

src/codegen/debug.rs

-                .expect("Type should exist at this stage");
-            let alignment = var_type.get_type_information().get_alignment(index).bits();
-            self.register_local_variable(variable, alignment, func);
+            self.register_local_variable(variable, 0, func);


do we still need the alignment argument? If the alignment for debuginfo is now always 0, I think we can remove it from the API

mhasel · 2025-03-17T14:18:04Z

src/codegen/debug.rs

            let location = &datatype.location;
            match type_info {
                DataTypeInformation::Struct { members, .. } => {
                    self.create_struct_type(name, members.as_slice(), index, location)
                }
                DataTypeInformation::Array { name, inner_type_name, dimensions, .. } => {
-                    self.create_array_type(name, inner_type_name, dimensions, size, alignment, index)
+                    self.create_array_type(name, inner_type_name, dimensions, size, Bytes::new(0), index)


mhasel · 2025-03-17T14:18:10Z

src/codegen/debug.rs

                }
                DataTypeInformation::Pointer { name, inner_type_name, .. } => {
-                    self.create_pointer_type(name, inner_type_name, size, alignment, index)
+                    self.create_pointer_type(name, inner_type_name, size, Bytes::new(0), index)


mhasel · 2025-03-17T14:18:15Z

src/codegen/debug.rs

@@ -671,7 +663,7 @@ impl<'ink> Debug<'ink> for DebugBuilder<'ink> {
                    let length = string_size
                        .as_int_value(index)
                        .map_err(|err| Diagnostic::codegen_error(err, SourceLocation::undefined()))?;
-                    self.create_string_type(name, length, *encoding, size, alignment, index)
+                    self.create_string_type(name, length, *encoding, size, Bytes::new(0), index)


ghaith · 2025-03-18T04:30:24Z

I have tried to debug a quick example and it seems we cannot remove the alignment in the debuginfo after all. I'm getting quite a few garbage values in the debugger.

TYPE struct_ : STRUCT
        inner: inner;
        inner_arr: ARRAY[0..2] OF inner;
        s : STRING := 'Hello';
        b : BOOL := TRUE;
        r : REAL := 3.1415;
        arr: ARRAY[0..2] OF STRING := ['aa', 'bb', 'cc'];
        i : INT := 42;
    END_STRUCT
END_TYPE

TYPE inner : STRUCT
        s : STRING := 'Hello';
        b : BOOL := TRUE;
        r : REAL := 3.1415;
        arr: ARRAY[0..2] OF STRING := ['aaaa', 'bbbb', 'cccc'];
        i : INT := 42;
    END_STRUCT
END_TYPE

FUNCTION main 
VAR
    st: struct_;
    s : STRING;
    b : BOOL;
    arr: ARRAY[0..2] OF STRING;
    i : INT;
END_VAR
    s := st.s;
    s := st.inner.s;
    b := st.b;
    b := st.inner.b;
    arr := st.arr;
    arr := st.inner.arr;
    i := st.i;
    i := st.inner.i;
    // arr := ['', '', ''];
    arr[0] := st.arr[0];
    arr[1] := st.inner.arr[1];
    arr[2] := st.inner.arr[2];

    printf('s: %s$N', REF(st.s));
    printf('s: %s$N', REF(st.inner.s));

    printf('arr at 0: %s$N', REF(st.arr[0]));
    printf('arr at 1: %s$N', REF(st.arr[1]));
    printf('arr at 2: %s$N', REF(st.arr[2]));

    printf('inner arr at 0: %s$N', REF(st.inner.arr[0]));
    printf('inner arr at 1: %s$N', REF(st.inner.arr[1]));
    printf('inner arr at 2: %s$N', REF(st.inner.arr[2]));

    printf('inner struct arr at 0, inner string arr at 0: %s$N', REF(st.inner_arr[0].arr[0]));
END_FUNCTION

plc <file> <printf.pli> -g -Onone --linker clang -o out gdb out

After stopping on a breakpoint in main, using info locals in gdb shows the following:

Breakpoint 1, main () at target/demo.st:42
42          s := st.s;
(gdb) info locals
st = {inner = {s = "Hello", '\000' <repeats 75 times>, b = true, r = 2.63775365e-30, arr = {"I@aaaa", '\000' <repeats 74 times>, "\000\000bbbb", '\000' <repeats 74 times>, "\000\000cccc", '\000' <repeats 74 times>}, i = 0}, inner_arr = {{s = "\000*", '\000' <repeats 78 times>,
      b = false, r = 0, arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}, i = 0}, {s = '\000' <repeats 80 times>, b = false, r = 0, arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}, i = 0}, {
      s = '\000' <repeats 80 times>, b = false, r = 0, arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}, i = 0}}, s = '\000' <repeats 20 times>, "Hello", '\000' <repeats 55 times>, b = false, r = 0, arr = {
    '\000' <repeats 15 times>, "\001\000\000V\016I@aa", '\000' <repeats 56 times>, '\000' <repeats 22 times>, "bb", '\000' <repeats 56 times>, '\000' <repeats 22 times>, "cc", '\000' <repeats 56 times>}, i = 0}
s = '\000' <repeats 80 times>
b = false
arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}
i = 0

After trying this on master, I've noticed that the output is not correct either (also tried before the latest commit in regards to END_XXX and initializer debug changes). Here's the gdb output on master:

Breakpoint 1, main () at target/demo.st:46
46          arr := st.arr;
(gdb) info locals
st = {inner = {s = "Hello", '\000' <repeats 75 times>, b = true, r = 3.1415, arr = {"aaaa", '\000' <repeats 76 times>, "bbbb", '\000' <repeats 76 times>, "cccc", '\000' <repeats 76 times>}, i = 42}, inner_arr = {{s = '\000' <repeats 80 times>, b = false, r = 0, arr = {
        '\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}, i = 0}, {s = '\000' <repeats 80 times>, b = false, r = 0, arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}, i = 0}, {s = '\000' <repeats 80 times>,
      b = false, r = 0, arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}, i = 0}}, s = "\000\000\000\000\000\000Hello", '\000' <repeats 69 times>, b = false, r = 0, arr = {"\000\001\000\000V\016I@aa", '\000' <repeats 70 times>,
    "\000\000\000\000\000\000\000\000bb", '\000' <repeats 70 times>, "\000\000\000\000\000\000\000\000cc", '\000' <repeats 70 times>}, i = 0}
s = "Hello", '\000' <repeats 75 times>
b = true
arr = {'\000' <repeats 80 times>, '\000' <repeats 80 times>, '\000' <repeats 80 times>}
i = 0

Notice that here the first floating point value is still correct, but soon after this statement is no longer true - we are back to being confronted with garbage. Yet another thing I've noticed is that deeply nested structs, in this case the array of the inner struct within the struct do not seem to be initialized correctly, since the printf output is correct for all the values in the above example except for the last one, which simply prints nothing (i.e. the string starts nul-terminated). So maybe the garbage values in the debugger are a false-positive due to the values not being initialized, we're going to have to look into this further.

I would recommend we look at clang's behavior in a similar setting, effectively the same application in C. This tells us how they are handling these situations.
Effectively: I think we need a solution between aligning some fields but giving default values to others..

abroooo marked this pull request as draft March 13, 2025 14:49

abroooo added 2 commits March 14, 2025 10:08

prepare alignment removal for dbg

bb5221f

fix tests

57c9755

abroooo force-pushed the abroooo/fix_dbg_alignment branch from f38a04c to 57c9755 Compare March 14, 2025 09:30

abroooo added 2 commits March 14, 2025 10:59

fix get_size

3327303

remove alignment

5fc589a

abroooo changed the title ~~prepare alignment removal for dbg~~ remove alignment for debug data Mar 14, 2025

abroooo marked this pull request as ready for review March 14, 2025 10:36

abroooo requested review from mhasel and volsa March 14, 2025 10:36

Merge branch 'master' into abroooo/fix_dbg_alignment

a0c1033

mhasel requested changes Mar 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove alignment for debug data #1434

remove alignment for debug data #1434

abroooo commented Mar 13, 2025 •

edited

Loading

mhasel commented Mar 17, 2025 •

edited

Loading

mhasel Mar 17, 2025

mhasel Mar 17, 2025

mhasel Mar 17, 2025

mhasel Mar 17, 2025

mhasel Mar 17, 2025

mhasel Mar 17, 2025

mhasel Mar 17, 2025

ghaith commented Mar 18, 2025 •

edited

Loading

remove alignment for debug data #1434

Are you sure you want to change the base?

remove alignment for debug data #1434

Conversation

abroooo commented Mar 13, 2025 • edited Loading

mhasel commented Mar 17, 2025 • edited Loading

mhasel Mar 17, 2025

Choose a reason for hiding this comment

mhasel Mar 17, 2025

Choose a reason for hiding this comment

mhasel Mar 17, 2025

Choose a reason for hiding this comment

mhasel Mar 17, 2025

Choose a reason for hiding this comment

mhasel Mar 17, 2025

Choose a reason for hiding this comment

mhasel Mar 17, 2025

Choose a reason for hiding this comment

mhasel Mar 17, 2025

Choose a reason for hiding this comment

ghaith commented Mar 18, 2025 • edited Loading

abroooo commented Mar 13, 2025 •

edited

Loading

mhasel commented Mar 17, 2025 •

edited

Loading

ghaith commented Mar 18, 2025 •

edited

Loading