[Merge] ~liushuyu-011/ubuntu/+source/sbsigntool:ubuntu/devel into ubuntu/+source/sbsigntool:ubuntu/devel

Zixing Liu mp+452020 at code.launchpad.net
Mon Sep 25 18:27:49 UTC 2023


The proposal to merge ~liushuyu-011/ubuntu/+source/sbsigntool:ubuntu/devel into ubuntu/+source/sbsigntool:ubuntu/devel has been updated.

Description changed to:

This merge proposal fixes an FTBFS in sbsigntool when building with GCC 13.

You can find the error message in this build log: https://launchpadlibrarian.net/688582735/buildlog_ubuntu-mantic-amd64.sbsigntool_0.9.4-3.1ubuntu2_BUILDING.txt.gz.

More specifically, this error:

```
../lib/ccan/ccan/talloc/talloc.c:336:35: error: ‘_170’ may be used uninitialized [-Werror=maybe-uninitialized]
  336 |         struct talloc_chunk *tc = talloc_chunk_from_ptr(ptr);
      |                                   ^
../lib/ccan/ccan/talloc/talloc.c: In function ‘main’:
../lib/ccan/ccan/talloc/talloc.c:115:36: note: by argument 1 of type ‘const void *’ to ‘talloc_chunk_from_ptr’ declared here
  115 | static inline struct talloc_chunk *talloc_chunk_from_ptr(const void *ptr)
      |                                    ^
```

Note that GCC did not name the variable using the variable name in the source code. This is because `_170` is a GCC GIMPLE name (artificial construct) generated during the LTO phase.

If you enable GCC debug checking by specifying `-fdump-tree-uninit` to the `lto1` driver, you will find the `ctx = talloc_zero(NULL, struct sync_context);` expression has been lowered into GIMPLE like this:

```
  tc_162 = malloc (152);
  pretmp_14 = locked;
  if (tc_162 == 0B)
    goto <bb 3>; [0.00%]
  else
    goto <bb 4>; [100.00%]

  <bb 3> [count: 0]:
  if (pretmp_14 != 0)
    goto <bb 6>; [0.00%]
  else
    goto <bb 107>; [100.00%]

  <bb 4> [local count: 246914340]:
  tc_162->size = 72;
  tc_162->flags = 3893685360;
  tc_162->destructor = 0B;
  tc_162->name = 0B;
  tc_162->refs = 0B;
  MEM <vector(2) long unsigned int> [(struct talloc_chunk * *)tc_162] = { 0, 0 };
  MEM <vector(2) long unsigned int> [(struct talloc_chunk * *)tc_162 + 16B] = { 0, 0 };
  _170 = tc_162 + 80;
  tc_142 = talloc_chunk_from_ptr (_170);
  tc_142->name = "struct sync_context";
```

... which you will see variable `_170` is initialized as `tc_162 + 80`. This is because `talloc` allocated memory contains a 80-byte header. The real data starts from `+80` offset.

Since the function body is already very large, GCC did not decide to inline the `talloc_chunk_from_ptr` call. From GCC's standpoint, `tc_162 + 80` is indeed uninitialized before usage (because GCC does not know what happened after the `talloc_chunk_from_ptr` call, which the function will re-adjust the pointer to point to the header).

So the proper solution would be forcing GCC to inline the `talloc_chunk_from_ptr` call, where it will start to understand that the accessed data is not `_170` (which is `tc_162 + 80`), but `tc_162 + 0` (which is correctly initialized).

For more details, see:
https://code.launchpad.net/~liushuyu-011/ubuntu/+source/sbsigntool/+git/sbsigntool/+merge/452020
-- 
Your team Ubuntu Sponsors is requested to review the proposed merge of ~liushuyu-011/ubuntu/+source/sbsigntool:ubuntu/devel into ubuntu/+source/sbsigntool:ubuntu/devel.




More information about the Ubuntu-sponsors mailing list