On Mon, Oct 11, 2021 at 11:25 PM Martin Sebor <mse...@gmail.com> wrote:
>
> The attached change extends GCC's warnings for out-of-bounds
> stores to cover atomic (and __sync) built-ins.
>
> Rather than hardcoding the properties of these built-ins just
> for the sake of the out-of-bounds detection, on the assumption
> that it might be useful for future optimizations as well, I took
> the approach of extending class attr_fnspec to express their
> special property that they encode the size of the access in their
> name.
>
> I also took the liberty of making attr_fnspec assignable (something
> the rest of my patch relies on), and updating some comments for
> the characters the class uses to encode function properties, based
> on my understanding of their purpose.
>
> Tested on x86_64-linux.
Hmm, so you place 'A' at an odd place (where the return value is specified),
but you do not actually specify the behavior on the return value. Shoudln't
+ 'A' specifies that the function atomically accesses a constant
+ 1 << N bytes where N is indicated by character 3+2i
maybe read
'A' specifies that the function returns the memory pointed to
by argument
one of size 1 << N bytes where N is indicated by
character 3 +2i accessed atomically
? I also wonder if it's necessary to constrain this to 'atomic' accesses
for the purpose of the patch and whether that detail could be omitted to
eventually make more use of it?
Likewise
+ '0'...'9' specifies the size of value written/read is given either
+ by the specified argument, or for atomic functions, by
+ 2 ^ N where N is the constant value denoted by the character
should mention (excluding '0') for the argument position.
/* length of the fn spec string. */
- const unsigned len;
+ unsigned len;
why that?
+ /* Return true of the function is an __atomic or __sync built-in. */
you didn't specify that for 'A' ...
+ bool
+ atomic_p () const
+ {
+ return str[0] == 'A';
+ }
+attr_fnspec
+atomic_builtin_fnspec (tree callee)
+{
+ switch (DECL_FUNCTION_CODE (callee))
+ {
+#define BUILTIN_ACCESS_SIZE_FNSPEC(N, lgsz) \
+ BUILT_IN_ATOMIC_LOAD_ ## N: \
+ return "Ap" "R" lgsz;
note that doing this for atomics makes those no longer a compiler barrier
for (aliased) loads and stores which means they are no longer a reliable
way to implement locks. That's a reason why I never pushed a
PTA/alias patch I have to open-code this.
Thus, do we really want to do this?
Richard.
>
> Martin
