> On Jun 4, 2024, at 17:55, David Malcolm <dmalc...@redhat.com> wrote: > > On Fri, 2024-05-31 at 13:11 +0000, Qing Zhao wrote: >> >> >>> On May 31, 2024, at 08:58, Richard Biener <rguent...@suse.de> >>> wrote: >>> >>> On Thu, 30 May 2024, Qing Zhao wrote: >>> >>>> Including the following changes: >>>> * The definition of the new internal function .ACCESS_WITH_SIZE >>>> in internal-fn.def. >>>> * C FE converts every reference to a FAM with a "counted_by" >>>> attribute >>>> to a call to the internal function .ACCESS_WITH_SIZE. >>>> (build_component_ref in c_typeck.cc) >>>> >>>> This includes the case when the object is statically allocated >>>> and >>>> initialized. >>>> In order to make this working, the routine digest_init in c- >>>> typeck.cc >>>> is updated to fold calls to .ACCESS_WITH_SIZE to its first >>>> argument >>>> when require_constant is TRUE. >>>> >>>> However, for the reference inside "offsetof", the "counted_by" >>>> attribute is >>>> ignored since it's not useful at all. >>>> (c_parser_postfix_expression in c/c-parser.cc) >>>> >>>> In addtion to "offsetof", for the reference inside operator >>>> "typeof" and >>>> "alignof", we ignore counted_by attribute too. >>>> >>>> When building ADDR_EXPR for the .ACCESS_WITH_SIZE in C FE, >>>> replace the call with its first argument. >>>> >>>> * Convert every call to .ACCESS_WITH_SIZE to its first argument. >>>> (expand_ACCESS_WITH_SIZE in internal-fn.cc) >>>> * Provide the utility routines to check the call is >>>> .ACCESS_WITH_SIZE and >>>> get the reference from the call to .ACCESS_WITH_SIZE. >>>> (is_access_with_size_p and get_ref_from_access_with_size in >>>> tree.cc) >>> >>> The middle-end parts of this revised patch are OK. >> >> Thanks a lot for the review. >> Will commit the patch set soon. > > [...snip...] > > Congratulations on getting this merged. > > FWIW I've started investigating adding support for the new attribute to > -fanalyzer (and am tracked this as PR analyzer/111567 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111567 ).
Thank you for starting looking at this. > > The docs for the attribute speak of the implied relationship between > the count field and size of the flex array, and say that: "It's the > user's responsibility to make sure the above requirements to be kept > all the time. Otherwise the compiler *reports warnings*, at the same > time, the results of the array bound sanitizer and the > '__builtin_dynamic_object_size' is undefined." (my emphasis). > > What are these warnings that are reported? I looked through > r15-944-gf824acd0e80754 through r15-948-g4c5bea7def1361 and I didn't > see any new warnings or test coverage for warnings (beyond misuing the > attribute). Sorry if I'm missing something obvious here. These warnings will be in the remaining work (I listed the remaining work in all versions except the last one): >>>> ******Remaining works: >>>> >>>> 6 Improve __bdos to use the counted_by info in whole-object size for the >>>> structure with FAM. >>>> 7 Emit warnings when the user breaks the requirments for the new >>>> counted_by attribute >>>> compilation time: -Wcounted-by >>>> run time: -fsanitizer=counted-by >>>> * The initialization to the size field should be done before the first >>>> reference to the FAM field. >>>> * the array has at least # of elements specified by the size field all >>>> the time during the program. With the current patches that have been committed, the warnings are not emitted. I believe that more analysis and more information are needed for these warnings to be effective, it might not be a trivial patch. More discussion is needed for emitting such warnings. > > Does anyone have examples of cases that -fanalyzer ought to warn for? At this moment, I don’t have concrete testing cases for this yet, but I can come up with several small examples and share with you in a later email. Qing > Presumably it would be helpful for the analyzer to report about code > paths in which the requirements are violated (but it may be that the > analyzer runs too late to do this...) > > Thanks > Dave >
