Bad bindings for typedef to incomplete struct after Clang 22 change

[zmodem]

After Clang's llvm/llvm-project#147835 we're seeing bad bindings (https://crbug.com/440975178). Here's a reduced example.

Unlike #3264 this doesn't involve any nested classes. Instead, this seems related to the removal of ElaboratedType nodes.

Header:

typedef struct cbb_st CBB;
struct cbb_st {
  int x;
};

Bindings before the Clang change:

pub type CBB = cbb_st;
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct cbb_st {
    pub x: ::std::os::raw::c_int,
}
[...]

After the Clang change:

pub type CBB = cbb_st;
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct cbb_st {
    pub _address: u8,
}
[...]

[zmodem]

Actually, lots of tests seem to be failing after that change.

With libclang built at the breaking change:

$ export LIBCLANG_PATH=/work/llvm-project/build/lib
$ cargo build
$ cargo test
[...]
failures:
    header_allowlist_file_hpp
    header_attribute_custom_h
    header_blocklist_file_hpp
    header_canonical_types_hpp
    header_class_nested_hpp
    header_constified_enum_module_overflow_hpp
    header_derive_custom_h
    header_derive_partialeq_pointer_hpp
    header_duplicated_namespaces_definitions_hpp
    header_forward_declared_struct_h
    header_issue_1281_h
    header_issue_372_hpp
    header_issue_410_hpp
    header_issue_544_stylo_creduce_2_hpp
    header_issue_584_stylo_template_analysis_panic_hpp
    header_issue_888_enum_var_decl_jump_hpp
    header_layout_cmdline_token_h
    header_nested_hpp
    header_nsbasehashtable_hpp
    header_sentry_defined_multiple_times_hpp
    header_struct_containing_forward_declared_struct_h
    header_typedef_pointer_overlap_h
    header_typeref_hpp
    header_vtable_recursive_sig_hpp

test result: FAILED. 580 passed; 24 failed; 0 ignored; 0 measured; 0 filtered out; finished in 3.09s

With libclang built at the revision before:

$ cargo test
[...]
failures:
    header_issue_544_stylo_creduce_2_hpp
    header_nsbasehashtable_hpp

test result: FAILED. 602 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 3.12s

[ajz34]

This issue has already affected rust's crate mpi (rsmpi/rsmpi#218) if using OpenMPI as MPI distribution.

The OpenMPI type ompi_status_public_t is exactly the same structure to the code you've presented
https://github.com/open-mpi/ompi/blob/91539d0b51c3a855dc103831bebee64d5be18187/ompi/include/mpi.h.in#L456-L483

typedef struct ompi_status_public_t MPI_Status;
struct ompi_status_public_t {
    int MPI_SOURCE;
    int MPI_TAG;
    int MPI_ERROR;
    int _cancelled;
    size_t _ucount;
};
typedef struct ompi_status_public_t ompi_status_public_t;

[zmodem]

Dumping some notes:

AST before:

$ build.before/bin/clang -cc1 -ast-dump /tmp/a.h
...
|-RecordDecl 0x555d17ca2470 </tmp/a.h:1:9, col:16> col:16 struct cbb_st
|-TypedefDecl 0x555d17ca2568 <col:1, col:23> col:23 CBB 'struct cbb_st'
| `-ElaboratedType 0x555d17ca2510 'struct cbb_st' sugar
|   `-RecordType 0x555d17ca24f0 'struct cbb_st'
|     `-Record 0x555d17ca25d8 'cbb_st'
`-RecordDecl 0x555d17ca25d8 prev 0x555d17ca2470 <line:2:1, line:4:1> line:2:8 struct cbb_st definition
  `-FieldDecl 0x555d17ca2670 <line:3:3, col:7> col:7 x 'int'

After:

$ build.after/bin/clang -cc1 -ast-dump /tmp/a.h
...
|-RecordDecl 0x5617cc9ad890 </tmp/a.h:1:9, col:16> col:16 struct cbb_st
|-TypedefDecl 0x5617cc9ad978 <col:1, col:23> col:23 CBB 'struct cbb_st'
| `-RecordType 0x5617cc9ad940 'struct cbb_st' owns_tag struct
|   `-Record 0x5617cc9ad890 'cbb_st'
`-RecordDecl 0x5617cc9ad9e8 prev 0x5617cc9ad890 <line:2:1, line:4:1> line:2:8 struct cbb_st definition
  `-FieldDecl 0x5617cc9ada80 <line:3:3, col:7> col:7 x 'int'

There are two differences:

1. The ElaboratedType has been folded into the RecordType ("owns_tag struct")
2. The RecordType used to refer to the struct definition (line 2:8), but now refers to the declaration in the typedef (line 1:16).

I think it's the second point that's causing our troubles.

Maybe that's this part of the Clang PR:

TagTypes can now point to the exact declaration found when producing these, as opposed to the previous situation of there only existing one TagType per entity.

---

Looking at the debug! logs (with the timestamps stripped):

$ RUST_LOG=bindgen LIBCLANG_PATH=/work/llvm-project/build.before/lib target/debug/bindgen /tmp/a.h 2>&1 | sed -Ee 's/\[2025.*Z//' > /tmp/before.txt
$ RUST_LOG=bindgen LIBCLANG_PATH=/work/llvm-project/build.after/lib target/debug/bindgen /tmp/a.h 2>&1 | sed -Ee 's/\[2025.*Z//' > /tmp/after.txt

Attached:
before.txt
after.txt

Our struct first shows up in the diff like this.

Before:

 DEBUG bindgen::ir::item] from_ty_or_ref_with_id: ItemId(1) Type(struct cbb_st, kind: Record, cconv: 100, decl: Cursor(cbb_st kind: StructDecl, loc: /tmp/a.h:2:8, usr: Some("c:@S@cbb_st")), canon: Cursor(cbb_st kind: StructDecl, loc: /tmp/a.h:1:16, usr: Some("c:@S@cbb_st"))), Cursor(cbb_st kind: StructDecl, loc: /tmp/a.h:1:16, usr: Some("c:@S@cbb_st")), None

After:

 DEBUG bindgen::ir::item] from_ty_or_ref_with_id: ItemId(1) Type(struct cbb_st, kind: Record, cconv: 100, decl: Cursor(cbb_st kind: StructDecl, loc: /tmp/a.h:1:16, usr: Some("c:@S@cbb_st")), canon: Cursor(cbb_st kind: StructDecl, loc: /tmp/a.h:1:16, usr: Some("c:@S@cbb_st"))), Cursor(cbb_st kind: StructDecl, loc: /tmp/a.h:1:16, usr: Some("c:@S@cbb_st")), None

I think that's from processing the decl in the typedef, but note how the cursor's decl refers to the incomplete vs complete type.

When we later run CompInfo::from_ty it's again looking at the incomplete type.

Before:

 DEBUG bindgen::ir::comp] CompInfo::from_ty(Struct, Cursor(cbb_st kind: StructDecl, loc: /tmp/a.h:2:8, usr: Some("c:@S@cbb_st")))

After:

 DEBUG bindgen::ir::comp] CompInfo::from_ty(Struct, Cursor(cbb_st kind: StructDecl, loc: /tmp/a.h:1:16, usr: Some("c:@S@cbb_st")))

[zmodem]

With this applied:

diff --git a/bindgen/ir/comp.rs b/bindgen/ir/comp.rs
index c0860f9f..b45a9dd8 100644
--- a/bindgen/ir/comp.rs
+++ b/bindgen/ir/comp.rs
@@ -1234,6 +1234,12 @@ impl CompInfo {
         );
 
         let mut cursor = ty.declaration();
+
+        // If there is a definition, that's what we want.
+        if let Some(def) = cursor.definition() {
+            cursor = def
+        }
+
         let mut kind = Self::kind_from_cursor(&cursor);
         if kind.is_err() {
             if let Some(location) = location {

I get the expected bindings, and a bunch of tests start passing (but some still fail):

$ export LIBCLANG_PATH=/work/llvm-project/build.after/lib
$ cargo test
[...]
failures:
    header_constified_enum_module_overflow_hpp
    header_issue_544_stylo_creduce_2_hpp
    header_nsbasehashtable_hpp
    header_typedef_pointer_overlap_h

test result: FAILED. 600 passed; 4 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.86s

[emilio]

Thanks for the investigation! I was just starting to look into it and there's already a PR, amazing.

Yeah, for reference this is a diff of the AST as bindgen sees it:

diff --git a/old.txt b/new.txt
index 904fa6f2..5b549cdd 100644
--- a/old.txt
+++ b/new.txt
@@ -18,36 +18,6 @@
  type.cconv = 100
  type.spelling = "struct cbb_st"
  type.is-variadic? false
-
- type.declaration.kind = structdecl
- type.declaration.spelling = "cbb_st"
- type.declaration.location = t.h:2:8
- type.declaration.is-definition? true
- type.declaration.is-declaration? true
- type.declaration.is-inlined-function? false
- type.declaration.usr = "c:@s@cbb_st"
-
- type.declaration.canonical.kind = structdecl
- type.declaration.canonical.spelling = "cbb_st"
- type.declaration.canonical.location = t.h:1:16
- type.declaration.canonical.is-definition? false
- type.declaration.canonical.is-declaration? true
- type.declaration.canonical.is-inlined-function? false
- type.declaration.canonical.usr = "c:@s@cbb_st"
-
- type.declaration.canonical.semantic-parent.kind = translationunit
- type.declaration.canonical.semantic-parent.spelling = "t.h"
- type.declaration.canonical.semantic-parent.location = builtin definitions
- type.declaration.canonical.semantic-parent.is-definition? false
- type.declaration.canonical.semantic-parent.is-declaration? false
- type.declaration.canonical.semantic-parent.is-inlined-function? false
-
- type.declaration.semantic-parent.kind = translationunit
- type.declaration.semantic-parent.spelling = "t.h"
- type.declaration.semantic-parent.location = builtin definitions
- type.declaration.semantic-parent.is-definition? false
- type.declaration.semantic-parent.is-declaration? false
- type.declaration.semantic-parent.is-inlined-function? false
 )
 (
  kind = typedefdecl
@@ -57,7 +27,7 @@
  is-declaration? true
  is-inlined-function? false
  usr = "c:t.h@t@cbb"
- typedef-type = elaborated
+ typedef-type = record
 
  semantic-parent.kind = translationunit
  semantic-parent.spelling = "t.h"
@@ -86,27 +56,12 @@
 
      referenced.kind = structdecl
      referenced.spelling = "cbb_st"
-     referenced.location = t.h:2:8
-     referenced.is-definition? true
+     referenced.location = t.h:1:16
+     referenced.is-definition? false
      referenced.is-declaration? true
      referenced.is-inlined-function? false
      referenced.usr = "c:@s@cbb_st"
 
-     referenced.canonical.kind = structdecl
-     referenced.canonical.spelling = "cbb_st"
-     referenced.canonical.location = t.h:1:16
-     referenced.canonical.is-definition? false
-     referenced.canonical.is-declaration? true
-     referenced.canonical.is-inlined-function? false
-     referenced.canonical.usr = "c:@s@cbb_st"
-
-     referenced.canonical.semantic-parent.kind = translationunit
-     referenced.canonical.semantic-parent.spelling = "t.h"
-     referenced.canonical.semantic-parent.location = builtin definitions
-     referenced.canonical.semantic-parent.is-definition? false
-     referenced.canonical.semantic-parent.is-declaration? false
-     referenced.canonical.semantic-parent.is-inlined-function? false
-
      referenced.semantic-parent.kind = translationunit
      referenced.semantic-parent.spelling = "t.h"
      referenced.semantic-parent.location = builtin definitions
@@ -121,27 +76,12 @@
 
      type.declaration.kind = structdecl
      type.declaration.spelling = "cbb_st"
-     type.declaration.location = t.h:2:8
-     type.declaration.is-definition? true
+     type.declaration.location = t.h:1:16
+     type.declaration.is-definition? false
      type.declaration.is-declaration? true
      type.declaration.is-inlined-function? false
      type.declaration.usr = "c:@s@cbb_st"
 
-     type.declaration.canonical.kind = structdecl
-     type.declaration.canonical.spelling = "cbb_st"
-     type.declaration.canonical.location = t.h:1:16
-     type.declaration.canonical.is-definition? false
-     type.declaration.canonical.is-declaration? true
-     type.declaration.canonical.is-inlined-function? false
-     type.declaration.canonical.usr = "c:@s@cbb_st"
-
-     type.declaration.canonical.semantic-parent.kind = translationunit
-     type.declaration.canonical.semantic-parent.spelling = "t.h"
-     type.declaration.canonical.semantic-parent.location = builtin definitions
-     type.declaration.canonical.semantic-parent.is-definition? false
-     type.declaration.canonical.semantic-parent.is-declaration? false
-     type.declaration.canonical.semantic-parent.is-inlined-function? false
-
      type.declaration.semantic-parent.kind = translationunit
      type.declaration.semantic-parent.spelling = "t.h"
      type.declaration.semantic-parent.location = builtin definitions
@@ -186,6 +126,21 @@
  type.spelling = "struct cbb_st"
  type.is-variadic? false
 
+ type.declaration.kind = structdecl
+ type.declaration.spelling = "cbb_st"
+ type.declaration.location = t.h:1:16
+ type.declaration.is-definition? false
+ type.declaration.is-declaration? true
+ type.declaration.is-inlined-function? false
+ type.declaration.usr = "c:@s@cbb_st"
+
+ type.declaration.semantic-parent.kind = translationunit
+ type.declaration.semantic-parent.spelling = "t.h"
+ type.declaration.semantic-parent.location = builtin definitions
+ type.declaration.semantic-parent.is-definition? false
+ type.declaration.semantic-parent.is-declaration? false
+ type.declaration.semantic-parent.is-inlined-function? false
+
     (
      kind = fielddecl
      spelling = "x"
@@ -237,11 +192,10 @@ pub type cbb = cbb_st;
 #[repr(c)]
 #[derive(debug, copy, clone)]
 pub struct cbb_st {
-    pub x: ::std::os::raw::c_int,
+    pub _address: u8,
 }
 #[allow(clippy::unnecessary_operation, clippy::identity_op)]
 const _: () = {
     ["size of cbb_st"][::std::mem::size_of::<cbb_st>() - 4usize];
     ["alignment of cbb_st"][::std::mem::align_of::<cbb_st>() - 4usize];
-    ["offset of field: cbb_st::x"][::std::mem::offset_of!(cbb_st, x) - 0usize];
 };