public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/107844] New: error: argument is not a field access for __builtin_preserve_field_info
@ 2022-11-23 17:41 james.hilliard1 at gmail dot com
2023-01-18 18:47 ` [Bug target/107844] " david.faust at oracle dot com
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: james.hilliard1 at gmail dot com @ 2022-11-23 17:41 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107844
Bug ID: 107844
Summary: error: argument is not a field access for
__builtin_preserve_field_info
Product: gcc
Version: 13.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: james.hilliard1 at gmail dot com
Target Milestone: ---
I'm seeing this error which does not occur in llvm for a bpf
test(test_core_reloc_existence.c) in bpf-next:
In file included from progs/test_core_reloc_existence.c:7:
progs/test_core_reloc_existence.c: In function 'test_core_existence':
/home/buildroot/bpf-next/tools/bpf/resolve_btfids/libbpf/include/bpf/bpf_core_read.h:132:9:
error: argument is not a field access
132 | __builtin_preserve_field_info(___bpf_field_ref(field),
BPF_FIELD_EXISTS)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
progs/test_core_reloc_existence.c:66:13: note: in expansion of macro
'bpf_core_field_exists'
66 | if (bpf_core_field_exists(struct core_reloc_existence, arr))
| ^~~~~~~~~~~~~~~~~~~~~
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/107844] error: argument is not a field access for __builtin_preserve_field_info
2022-11-23 17:41 [Bug target/107844] New: error: argument is not a field access for __builtin_preserve_field_info james.hilliard1 at gmail dot com
@ 2023-01-18 18:47 ` david.faust at oracle dot com
2023-01-18 18:58 ` pinskia at gcc dot gnu.org
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: david.faust at oracle dot com @ 2023-01-18 18:47 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107844
--- Comment #1 from David Faust <david.faust at oracle dot com> ---
Looks like this is a result of the combination of how the bpf_core_field_exists
macro is defined and some sort of optimization(?) happening in the C frontend.
Consider:
struct S {
unsigned short x;
char c[12];
};
int foo () {
int has_c = bpf_core_field_exists (struct S, c);
return has_c;
}
The bpf_core_field_exists macro expands to:
int has_c =
__builtin_preserve_field_info((((typeof(struct S) *)0)->c),
BPF_RELO_FIELD_EXISTS);
For some reason, for array members specifically, this construction results in
the tree for the parameter when resolving the builtin being simply:
(gdb) pt param
<integer_cst 0x7ffff7531b88 type <pointer_type 0x7ffff741bc78> constant 2>
i.e. a pointer to 0x2, the offset of 'c' in a 'struct S' mapped at 0x0. For
non-array members, 'param' is some kind of <component_ref> as I'd expect.
This gives us two problems:
a) We cannot distinguish an array member from a non-member constant-value
pointer.
b) We cannot correctly compute information for the CO-RE relocation.
For example, in this case BPF_RELO_FIELD_BYTE_SIZE will be calculated as 8
(size of pointer), not 12 (size of the array).
I'm not sure how to resolve this and support the existing helper macros in the
kernel/libbpf. Might need some change in the C frontend, or to somehow pass
more
information to the target_resolve_overloaded_builtin hook...
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/107844] error: argument is not a field access for __builtin_preserve_field_info
2022-11-23 17:41 [Bug target/107844] New: error: argument is not a field access for __builtin_preserve_field_info james.hilliard1 at gmail dot com
2023-01-18 18:47 ` [Bug target/107844] " david.faust at oracle dot com
@ 2023-01-18 18:58 ` pinskia at gcc dot gnu.org
2023-01-18 19:44 ` david.faust at oracle dot com
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-01-18 18:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107844
--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The reason why is folded is because some folks use the null pointer for
offsetof (previously before GCC added __builtin_offsetof).
I wonder if you could use __builtin_offsetof here.
I also curious how this is implemented in clang ...
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/107844] error: argument is not a field access for __builtin_preserve_field_info
2022-11-23 17:41 [Bug target/107844] New: error: argument is not a field access for __builtin_preserve_field_info james.hilliard1 at gmail dot com
2023-01-18 18:47 ` [Bug target/107844] " david.faust at oracle dot com
2023-01-18 18:58 ` pinskia at gcc dot gnu.org
@ 2023-01-18 19:44 ` david.faust at oracle dot com
2023-01-18 20:19 ` pinskia at gcc dot gnu.org
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: david.faust at oracle dot com @ 2023-01-18 19:44 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107844
--- Comment #3 from David Faust <david.faust at oracle dot com> ---
Thanks for the info Andrew. I'll look at __builtin_offsetof.
As for the implementation in clang, I can point to some bits relevant to
the builtin itself:
llvm-project/clang/lib/CodeGen/CGBuiltin.cpp
CodeGenFunction::EmitBPFBuiltinExpr ()
llvm-project/llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
BPFAbstractMemberAccess::GetFieldInfo ()
But I am less familiar with the surrounding machinery such as their
parsing and type systems..
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/107844] error: argument is not a field access for __builtin_preserve_field_info
2022-11-23 17:41 [Bug target/107844] New: error: argument is not a field access for __builtin_preserve_field_info james.hilliard1 at gmail dot com
` (2 preceding siblings ...)
2023-01-18 19:44 ` david.faust at oracle dot com
@ 2023-01-18 20:19 ` pinskia at gcc dot gnu.org
2023-01-18 21:16 ` david.faust at oracle dot com
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-01-18 20:19 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107844
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Ever confirmed|0 |1
Last reconfirmed| |2023-01-18
--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to David Faust from comment #3)
> Thanks for the info Andrew. I'll look at __builtin_offsetof.
>
> As for the implementation in clang, I can point to some bits relevant to
> the builtin itself:
> llvm-project/clang/lib/CodeGen/CGBuiltin.cpp
> CodeGenFunction::EmitBPFBuiltinExpr ()
>
> llvm-project/llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
> BPFAbstractMemberAccess::GetFieldInfo ()
>
> But I am less familiar with the surrounding machinery such as their
> parsing and type systems..
So I looked (First off I am shocked they don't have target functions to handle
the builtins and every target builtin is handled in that file seems wrong), and
you are handed the AST before folding. This is different from GCC where it is
you are handed it after folding.
So I think we need some special handling in the c (and C++) parser to handle
this. I suspect we want to do the full handling of the builtin
(bpf_core_field_exists) in the parser rather than the macro expanded view of it
too. Similar to how offsetof is handled ...
Of course this will need some modifications to the bpf headers too. And that
solves some other issues too.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/107844] error: argument is not a field access for __builtin_preserve_field_info
2022-11-23 17:41 [Bug target/107844] New: error: argument is not a field access for __builtin_preserve_field_info james.hilliard1 at gmail dot com
` (3 preceding siblings ...)
2023-01-18 20:19 ` pinskia at gcc dot gnu.org
@ 2023-01-18 21:16 ` david.faust at oracle dot com
2023-04-19 16:33 ` jemarch at gcc dot gnu.org
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: david.faust at oracle dot com @ 2023-01-18 21:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107844
--- Comment #5 from David Faust <david.faust at oracle dot com> ---
(In reply to Andrew Pinski from comment #4)
> (In reply to David Faust from comment #3)
> > Thanks for the info Andrew. I'll look at __builtin_offsetof.
> >
> > As for the implementation in clang, I can point to some bits relevant to
> > the builtin itself:
> > llvm-project/clang/lib/CodeGen/CGBuiltin.cpp
> > CodeGenFunction::EmitBPFBuiltinExpr ()
> >
> > llvm-project/llvm/lib/Target/BPF/BPFAbstractMemberAccess.cpp
> > BPFAbstractMemberAccess::GetFieldInfo ()
> >
> > But I am less familiar with the surrounding machinery such as their
> > parsing and type systems..
>
> So I looked (First off I am shocked they don't have target functions to
> handle the builtins and every target builtin is handled in that file seems
> wrong), and you are handed the AST before folding. This is different from
> GCC where it is you are handed it after folding.
Aha! I had never realized this difference until now. Thanks for pointing
that out!
>
> So I think we need some special handling in the c (and C++) parser to handle
> this. I suspect we want to do the full handling of the builtin
> (bpf_core_field_exists) in the parser rather than the macro expanded view of
> it too. Similar to how offsetof is handled ...
> Of course this will need some modifications to the bpf headers too. And that
> solves some other issues too.
Yes, I see. I'll have to study the parser a little since I have not touched
it before, but the approach makes sense.
I wonder if it would be feasible and/or worthwhile to add some sort of
TARGET_PARSE_BUILTIN hook to enable this sort of handling for any other
targets which may want it..? Maybe that can wait.
In any case, thank you very much for the suggestions.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/107844] error: argument is not a field access for __builtin_preserve_field_info
2022-11-23 17:41 [Bug target/107844] New: error: argument is not a field access for __builtin_preserve_field_info james.hilliard1 at gmail dot com
` (4 preceding siblings ...)
2023-01-18 21:16 ` david.faust at oracle dot com
@ 2023-04-19 16:33 ` jemarch at gcc dot gnu.org
2023-08-03 18:50 ` cvs-commit at gcc dot gnu.org
2023-11-27 6:40 ` jemarch at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: jemarch at gcc dot gnu.org @ 2023-04-19 16:33 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107844
Jose E. Marchesi <jemarch at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |cupertino.miranda at oracle dot co
| |m
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/107844] error: argument is not a field access for __builtin_preserve_field_info
2022-11-23 17:41 [Bug target/107844] New: error: argument is not a field access for __builtin_preserve_field_info james.hilliard1 at gmail dot com
` (5 preceding siblings ...)
2023-04-19 16:33 ` jemarch at gcc dot gnu.org
@ 2023-08-03 18:50 ` cvs-commit at gcc dot gnu.org
2023-11-27 6:40 ` jemarch at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-08-03 18:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107844
--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Cupertino Miranda <cupermir@gcc.gnu.org>:
https://gcc.gnu.org/g:e0a81559c198153923f0a1a3be7c25df545f3964
commit r14-2962-ge0a81559c198153923f0a1a3be7c25df545f3964
Author: Cupertino Miranda <cupertino.miranda@oracle.com>
Date: Thu Apr 6 15:22:48 2023 +0100
bpf: Implementation of BPF CO-RE builtins
This patch updates the support for the BPF CO-RE builtins
__builtin_preserve_access_index and __builtin_preserve_field_info,
and adds support for the CO-RE builtins __builtin_btf_type_id,
__builtin_preserve_type_info and __builtin_preserve_enum_value.
These CO-RE relocations are now converted to __builtin_core_reloc which
abstracts all of the original builtins in a polymorphic relocation
specific builtin.
The builtin processing is now split in 2 stages, the first (pack) is
executed right after the front-end and the second (process) right before
the asm output.
In expand pass the __builtin_core_reloc is converted to a
unspec:UNSPEC_CORE_RELOC rtx entry.
The data required to process the builtin is now collected in the packing
stage (after front-end), not allowing the compiler to optimize any of
the relevant information required to compose the relocation when
necessary.
At expansion, that information is recovered and CTF/BTF is queried to
construct the information that will be used in the relocation.
At this point the relocation is added to specific section and the
builtin is expanded to the expected default value for the builtin.
In order to process __builtin_preserve_enum_value, it was necessary to
hook the front-end to collect the original enum value reference.
This is needed since the parser folds all the enum values to its
integer_cst representation.
More details can be found within the core-builtins.cc.
Regtested in host x86_64-linux-gnu and target bpf-unknown-none.
gcc/ChangeLog:
PR target/107844
PR target/107479
PR target/107480
PR target/107481
* config.gcc: Added core-builtins.cc and .o files.
* config/bpf/bpf-passes.def: Removed file.
* config/bpf/bpf-protos.h (bpf_add_core_reloc,
bpf_replace_core_move_operands): New prototypes.
* config/bpf/bpf.cc (enum bpf_builtins, is_attr_preserve_access,
maybe_make_core_relo, bpf_core_field_info, bpf_core_compute,
bpf_core_get_index, bpf_core_new_decl, bpf_core_walk,
bpf_is_valid_preserve_field_info_arg, is_attr_preserve_access,
handle_attr_preserve, pass_data_bpf_core_attr, pass_bpf_core_attr):
Removed.
(def_builtin, bpf_expand_builtin, bpf_resolve_overloaded_builtin):
Changed.
* config/bpf/bpf.md (define_expand mov<MM:mode>): Changed.
(mov_reloc_core<mode>): Added.
* config/bpf/core-builtins.cc (struct cr_builtin, enum
cr_decision struct cr_local, struct cr_final, struct
core_builtin_helpers, enum bpf_plugin_states): Added types.
(builtins_data, core_builtin_helpers, core_builtin_type_defs):
Added variables.
(allocate_builtin_data, get_builtin-data, search_builtin_data,
remove_parser_plugin, compare_same_kind, compare_same_ptr_expr,
compare_same_ptr_type, is_attr_preserve_access, core_field_info,
bpf_core_get_index, compute_field_expr,
pack_field_expr_for_access_index,
pack_field_expr_for_preserve_field,
process_field_expr, pack_enum_value, process_enum_value, pack_type,
process_type, bpf_require_core_support, make_core_relo, read_kind,
kind_access_index, kind_preserve_field_info, kind_enum_value,
kind_type_id, kind_preserve_type_info,
get_core_builtin_fndecl_for_type,
bpf_handle_plugin_finish_type, bpf_init_core_builtins,
construct_builtin_core_reloc, bpf_resolve_overloaded_core_builtin,
bpf_expand_core_builtin, bpf_add_core_reloc,
bpf_replace_core_move_operands): Added functions.
* config/bpf/core-builtins.h (enum bpf_builtins): Added.
(bpf_init_core_builtins, bpf_expand_core_builtin,
bpf_resolve_overloaded_core_builtin): Added functions.
* config/bpf/coreout.cc (struct bpf_core_extra): Added.
(bpf_core_reloc_add, output_asm_btfext_core_reloc): Changed.
* config/bpf/coreout.h (bpf_core_reloc_add) Changed prototype.
* config/bpf/t-bpf: Added core-builtins.o.
* doc/extend.texi: Added documentation for new BPF builtins.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug target/107844] error: argument is not a field access for __builtin_preserve_field_info
2022-11-23 17:41 [Bug target/107844] New: error: argument is not a field access for __builtin_preserve_field_info james.hilliard1 at gmail dot com
` (6 preceding siblings ...)
2023-08-03 18:50 ` cvs-commit at gcc dot gnu.org
@ 2023-11-27 6:40 ` jemarch at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: jemarch at gcc dot gnu.org @ 2023-11-27 6:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107844
Jose E. Marchesi <jemarch at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
--- Comment #7 from Jose E. Marchesi <jemarch at gcc dot gnu.org> ---
This is fixed by the commit above.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-11-27 6:40 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-23 17:41 [Bug target/107844] New: error: argument is not a field access for __builtin_preserve_field_info james.hilliard1 at gmail dot com
2023-01-18 18:47 ` [Bug target/107844] " david.faust at oracle dot com
2023-01-18 18:58 ` pinskia at gcc dot gnu.org
2023-01-18 19:44 ` david.faust at oracle dot com
2023-01-18 20:19 ` pinskia at gcc dot gnu.org
2023-01-18 21:16 ` david.faust at oracle dot com
2023-04-19 16:33 ` jemarch at gcc dot gnu.org
2023-08-03 18:50 ` cvs-commit at gcc dot gnu.org
2023-11-27 6:40 ` jemarch at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).