Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attribute btf_type_tag makes constness of pointers disappear #2244

Closed
ojeda opened this issue Jul 23, 2022 · 4 comments · Fixed by #2245
Closed

Attribute btf_type_tag makes constness of pointers disappear #2244

ojeda opened this issue Jul 23, 2022 · 4 comments · Fixed by #2245

Comments

@ojeda
Copy link
Contributor

ojeda commented Jul 23, 2022

From Rust-for-Linux/linux#835, reported by @YakoYakoYokuYoku.

I have not checked yet whether this is due to libclang, but nevertheless I wanted to have the issue opened here.

Input C/C++ Header

void f(const char __attribute__((btf_type_tag("a"))) *);

Bindgen Invocation

$ bindgen input.h

Actual Results

extern "C" {
    pub fn f(arg1: *mut ::std::os::raw::c_char);
}

Expected Results

extern "C" {
    pub fn f(arg1: *const ::std::os::raw::c_char);
}
@emilio
Copy link
Contributor

emilio commented Jul 25, 2022

It seems clang treats these a bit differently internally. For that function we get:

[2022-07-25T08:57:49Z DEBUG bindgen::ir::item] Item::from_ty_with_id: ItemId(11)
    	ty = Type(char const * __attribute__((btf_type_tag)), kind: Pointer, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)),
    	location = Cursor( kind: ParmDecl, loc: t.h:1:55, usr: None)
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] builtin_or_resolved_ty: Type(char const * __attribute__((btf_type_tag)), kind: Pointer, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)), Some(Cursor( kind: ParmDecl, loc: t.h:1:55, usr: None)), ItemId(11), None
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] Not resolved, maybe builtin?
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] builtin_or_resolved_ty: Type(char const * __attribute__((btf_type_tag)), kind: Pointer, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)), Some(Cursor( kind: ParmDecl, loc: t.h:1:55, usr: None)), ItemId(11), None
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] Not resolved, maybe builtin?
[2022-07-25T08:57:49Z DEBUG bindgen::ir::ty] from_clang_ty: ItemId(11), ty: Type(char const * __attribute__((btf_type_tag)), kind: Pointer, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)), loc: Cursor( kind: ParmDecl, loc: t.h:1:55, usr: None)
[2022-07-25T08:57:49Z DEBUG bindgen::ir::ty] currently_parsed_types: []
[2022-07-25T08:57:49Z DEBUG bindgen::ir::item] from_ty_or_ref_with_id: ItemId(12) Type(char, kind: Char_S, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)), Cursor( kind: ParmDecl, loc: t.h:1:55, usr: None), None
[2022-07-25T08:57:49Z DEBUG bindgen::ir::item] refs already collected, resolving directly
[2022-07-25T08:57:49Z DEBUG bindgen::ir::item] Item::from_ty_with_id: ItemId(12)
    	ty = Type(char, kind: Char_S, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)),
    	location = Cursor( kind: ParmDecl, loc: t.h:1:55, usr: None)
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] builtin_or_resolved_ty: Type(char, kind: Char_S, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)), Some(Cursor( kind: ParmDecl, loc: t.h:1:55, usr: None)), ItemId(12), None
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] Not resolved, maybe builtin?
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] add_builtin_item: item = Item { id: ItemId(13), local_id: LazyCell { inner: UnsafeCell { .. } }, next_child_local_id: Cell { value: 1 }, canonical_name: LazyCell { inner: UnsafeCell { .. } }, path_for_allowlisting: LazyCell { inner: UnsafeCell { .. } }, comment: None, annotations: Annotations { opaque: false, hide: false, use_instead_of: None, disallow_copy: false, disallow_debug: false, disallow_default: false, must_use_type: false, private_fields: None, accessor_kind: None, constify_enum_variant: false, derives: [] }, parent_id: ItemId(0), kind: Type(Type { name: Some("char"), layout: Some(Layout { size: 1, align: 1, packed: false }), kind: Int(Char { is_signed: true }), is_const: false }), location: Some(builtin definitions) }
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] add_item_to_module: adding ItemId(13) as child of parent module ItemId(0)
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] BindgenContext::add_item(Item { id: ItemId(11), local_id: LazyCell { inner: UnsafeCell { .. } }, next_child_local_id: Cell { value: 1 }, canonical_name: LazyCell { inner: UnsafeCell { .. } }, path_for_allowlisting: LazyCell { inner: UnsafeCell { .. } }, comment: None, annotations: Annotations { opaque: false, hide: false, use_instead_of: None, disallow_copy: false, disallow_debug: false, disallow_default: false, must_use_type: false, private_fields: None, accessor_kind: None, constify_enum_variant: false, derives: [] }, parent_id: ItemId(0), kind: Type(Type { name: None, layout: Some(Layout { size: 8, align: 8, packed: false }), kind: Pointer(TypeId(ItemId(13))), is_const: false }), location: Some(t.h:1:55) }, declaration: Some(Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)), loc: Some(Cursor( kind: ParmDecl, loc: t.h:1:55, usr: None))
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] add_item_to_module: adding ItemId(11) as child of parent module ItemId(0)
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] Invalid declaration Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None) found for type Type { name: None, layout: Some(Layout { size: 8, align: 8, packed: false }), kind: Pointer(TypeId(ItemId(13))), is_const: false }

If I remove the attribute:

[2022-07-25T08:57:49Z DEBUG bindgen::ir::item] Item::from_ty_with_id: ItemId(14)
    	ty = Type(const char *, kind: Pointer, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)),
    	location = Cursor( kind: ParmDecl, loc: t.h:2:19, usr: None)
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] builtin_or_resolved_ty: Type(const char *, kind: Pointer, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)), Some(Cursor( kind: ParmDecl, loc: t.h:2:19, usr: None)), ItemId(14), None
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] Not resolved, maybe builtin?
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] builtin_or_resolved_ty: Type(const char *, kind: Pointer, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)), Some(Cursor( kind: ParmDecl, loc: t.h:2:19, usr: None)), ItemId(14), None
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] Not resolved, maybe builtin?
[2022-07-25T08:57:49Z DEBUG bindgen::ir::ty] from_clang_ty: ItemId(14), ty: Type(const char *, kind: Pointer, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)), loc: Cursor( kind: ParmDecl, loc: t.h:2:19, usr: None)
[2022-07-25T08:57:49Z DEBUG bindgen::ir::ty] currently_parsed_types: []
[2022-07-25T08:57:49Z DEBUG bindgen::ir::item] from_ty_or_ref_with_id: ItemId(15) Type(const char, kind: Char_S, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)), Cursor( kind: ParmDecl, loc: t.h:2:19, usr: None), None
[2022-07-25T08:57:49Z DEBUG bindgen::ir::item] refs already collected, resolving directly
[2022-07-25T08:57:49Z DEBUG bindgen::ir::item] Item::from_ty_with_id: ItemId(15)
    	ty = Type(const char, kind: Char_S, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)),
    	location = Cursor( kind: ParmDecl, loc: t.h:2:19, usr: None)
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] builtin_or_resolved_ty: Type(const char, kind: Char_S, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)), Some(Cursor( kind: ParmDecl, loc: t.h:2:19, usr: None)), ItemId(15), None
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] Not resolved, maybe builtin?
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] add_builtin_item: item = Item { id: ItemId(16), local_id: LazyCell { inner: UnsafeCell { .. } }, next_child_local_id: Cell { value: 1 }, canonical_name: LazyCell { inner: UnsafeCell { .. } }, path_for_allowlisting: LazyCell { inner: UnsafeCell { .. } }, comment: None, annotations: Annotations { opaque: false, hide: false, use_instead_of: None, disallow_copy: false, disallow_debug: false, disallow_default: false, must_use_type: false, private_fields: None, accessor_kind: None, constify_enum_variant: false, derives: [] }, parent_id: ItemId(0), kind: Type(Type { name: Some("const char"), layout: Some(Layout { size: 1, align: 1, packed: false }), kind: Int(Char { is_signed: true }), is_const: true }), location: Some(builtin definitions) }
[2022-07-25T08:57:49Z DEBUG bindgen::ir::context] add_item_to_module: adding ItemId(16) as child of parent module ItemId(0)

In particular, it seems clang_getPointeeType returns a different type for this case (and loses the constness). Not sure if working around it in bindgen is doable / easy yet.

@emilio
Copy link
Contributor

emilio commented Jul 25, 2022

Seems constness is completely lost at the libclang level :'(

From a quick printf like this:

diff --git a/src/ir/ty.rs b/src/ir/ty.rs
index d573408c..13701e28 100644
--- a/src/ir/ty.rs
+++ b/src/ir/ty.rs
@@ -1032,6 +1032,7 @@ impl Type {
                 CXType_MemberPointer |
                 CXType_Pointer => {
                     let pointee = ty.pointee_type().unwrap();
+                    println!("{:?} -> {:?} is_const {:?} -> {:?}", ty, pointee, ty.is_const(), pointee.is_const());
                     let inner =
                         Item::from_ty_or_ref(pointee, location, None, ctx);
                     TypeKind::Pointer(inner)
Type(char const * __attribute__((btf_type_tag)), kind: Pointer, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)) -> Type(char, kind: Char_S, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)) is_const false -> false
Type(const char *, kind: Pointer, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)) -> Type(const char, kind: Char_S, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)) is_const false -> true

However from a clang ast dump with --emit-clang-ast, it seems we could use the canonical type here:

     type.kind = Pointer
     type.cconv = 100
     type.spelling = "char const * __attribute__((btf_type_tag))"
     type.is-variadic? false

     type.canonical.kind = Pointer
     type.canonical.cconv = 100
     type.canonical.spelling = "const char *"
     type.canonical.is-variadic? false

     type.canonical.pointee.kind = Char_S
     type.canonical.pointee.cconv = 100
     type.canonical.pointee.spelling = "const char"
     type.canonical.pointee.is-variadic? false

     type.pointee.kind = Char_S
     type.pointee.cconv = 100
     type.pointee.spelling = "char"
     type.pointee.is-variadic? false

emilio added a commit that referenced this issue Jul 25, 2022
Using the canonical type fixes this but changes the output of some tests
(in particular, pointer to typedefs now point to the underlying type).

That seems somewhat acceptable tho, and shouldn't be a correctness
regression (so this patch should be a correctness improvement).

Fixes #2244
emilio added a commit that referenced this issue Jul 25, 2022
Using the canonical type fixes this but changes the output of some tests
(in particular, pointer to typedefs now point to the underlying type).
So do this only in known-bad cases.

Fixes #2244
emilio added a commit that referenced this issue Jul 25, 2022
Using the canonical type fixes this but changes the output of some tests
(in particular, pointer to typedefs now point to the underlying type).
So do this only in known-bad cases.

Fixes #2244
emilio added a commit that referenced this issue Jul 25, 2022
Using the canonical type fixes this but changes the output of some tests
(in particular, pointer to typedefs now point to the underlying type).
So do this only in known-bad cases.

Fixes #2244
@nickdesaulniers
Copy link
Contributor

@ojeda I think you can drop the workaround now from kbuild if you upgrade to a newer release of bindgen that has this fix?

@nickdesaulniers
Copy link
Contributor

@ojeda rust/bindgen_parameters also contains a comment about 2aed6b0 in 0.59.2. Should the minimum version of bindgen be raised for these 2 issues?

aatifsyed pushed a commit to aatifsyed/rust-bindgen that referenced this issue Sep 22, 2022
Using the canonical type fixes this but changes the output of some tests
(in particular, pointer to typedefs now point to the underlying type).
So do this only in known-bad cases.

Fixes rust-lang#2244
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants