On Tue, 22 Nov 2011, Andrew MacLeod wrote:

>     int n = get_atomic_generic_size (loc, function, params);
>   
> +   /* Size of 0 is an error condition.  */
> +   if (n == 0)
> +     {
> +       *new_return = error_mark_node;
> +       return true;
> +     }

As far as I can see, get_atomic_generic_size can return 0 in various cases 
where it did not print an error message:

* The "default" case in the initial switch statement.  If this is not 
actually reachable, use gcc_unreachable () there.

* If TYPE_SIZE_UNIT if the pointer target type is zero.  This could be the 
case for empty structures or zero-size arrays (both GNU extensions).  
Logically there's nothing wrong with atomic operations on such a zero-size 
object (they should evaluate all operands for their side effects and then 
need do nothing more).  But if you don't want to make them work now, there 
should be a sorry () call and a PR for making them work.

In looking at that function, I also see another unrelated problem: it has 
a statement "return MEMMODEL_SEQ_CST;" (i.e. return 5;) but is otherwise 
meant to be returning a size not a memory model, so something doesn't make 
sense there.  A second unrelated problem:

void f (int n) { int a[n], b[n]; __atomic_load (&a, &b, __ATOMIC_SEQ_CST); }

ICEs in tree_low_cst.  C1X doesn't require atomic operations on 
variable-length types - arrays can't be _Atomic-qualified and atomic 
operations are only required on atomic types - but there should be a 
proper error not an ICE.

It seems a bad idea to put error_mark_node in the IR for code that has not 
generated an error.  So maybe you need to change get_atomic_generic_size's 
interface so that errors can be unambiguously distinguished from zero-size 
objects - and make the comment above that function explicitly say that on 
error return it has output an error message.

Finally, the patch needs a testcase added to the testsuite for the case 
reported in the PR as ICEing - and for any other problems found and fixed 
in the process.

-- 
Joseph S. Myers
jos...@codesourcery.com

Reply via email to