Something more to be concerned about

Go To Last Post
20 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Just when we though we had atomicity and volatile under control......

http://www.ganssle.com/articles/...

and the article it refers to:

www.cs.utah.edu/~regehr/papers/e...

AVR and GCC are mentioned.

What worried me was para 2.1 with the example of the volatile flag being re-ordered such that the intent was lost.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hmm.. I thought we had a link to that article earlier this year, but I can't locate it. I'm pretty sure I've browsed the article earlier though..

Note the acknowledgement of Eric Weddington contributing to the article (I seem to recall that also having been mentioned here before - it might even be that EW himself linked to this article previously).

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm pretty sure I was the one who posted the previous link after Eric told me about it.

EDIT: yup this:

https://www.avrfreaks.net/index.p...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Realising what to search for makes all the difference - a search for "regehr" alone yields several good hits. The paper has been linked to several times before:

Oldest by thone of the suthors himself: https://www.avrfreaks.net/index.p...

Then by Ossi, in the thread that was swimming around in my secondary carbon-based storage: https://www.avrfreaks.net/index.p...

You come in as honorable 3rd, Cliff.. :)

OT: Searching for "regehr" you also will find another contribution to the AVR community - his stack tool.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Regehr reported a bug that lived 9 years long unnoticed in avr-gcc. Try this:

#include 

char c = 42;

void __attribute__((noinline,noclone))
pr39633 (char a)
{
  a >>= 7;
  if (a)
    c = a;
}

int main()
{
  pr39633 (6);

  if (c != 42)
    abort();
    
  return 0;
}

With avr-gcc < 4.6.2 it will probably run on abort()

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I don't get it? That code builds and works as expected in latest WinAVR which is just 4.3.3.

When would 6>>7 not be 0?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The work done by John Regehr is very important, and fairly well known by now in compiler circles. There has been improvement over time. Yes, certainly there is more work to do.

And yes I pointed out the article earlier when it first came out. Yes, I had the opportunity to review the article and gave a little feedback before he published it.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I always was a little slow!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
I don't get it? That code builds and works as expected in latest WinAVR which is just 4.3.3.

When would 6>>7 not be 0?


Just tried it with avr-gcc-4.3.3 from WinAVR-20100110.

char c = 42; 

void pr39633 (char a) 
{ 
  a >>= 7; 
  if (a) 
    c = a; 
}

The code compiles to wrong code with -Os:

pr39633:
	lsl r24
	sbc r24,r24
	breq .L9
	sts c,r24
.L9:
	ret

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

SprinterSB wrote:

The code compiles to wrong code with -Os:

pr39633:
	lsl r24
	sbc r24,r24
	breq .L9
	sts c,r24
.L9:
	ret

This code was fully correct.

Only on a negative number "a >>= 7;" was -1 (0xFF), otherwise 0.

Peter

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

danni wrote:
SprinterSB wrote:
The code compiles to wrong code with -Os:

pr39633:
	lsl r24
	sbc r24,r24
	breq .L9
	sts c,r24
.L9:
	ret

This code was fully correct.

SBC propagates Z-flag from LSL. The code must read
pr39633:
	lsl r24
	sbc r24,r24
	tst r24
	breq .L9
	sts c,r24
.L9:
	ret

instead or, at your option,

pr39633:
	lsl r24
	sbc r24,r24
	cpse r24, __zero_reg__
	sts c,r24
	ret

if the ISA has no skip erratum.

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

The code compiles to wrong code with -Os:

I'm just trying to figure out how my WinAVR (4.3.3) with -Os is producing:

.global	pr39633
	.type	pr39633, @function
pr39633:
.LFB2:
.LM1:
.LVL0:
/* prologue: function */
/* frame size = 0 */
.LM2:
	rol r24
	clr r24
	rol r24
.LVL1:
.LM3:
	breq .L3
.LM4:
	sts c,r24
.L3:
	ret
.LFE2:
	.size	pr39633, .-pr39633

(build target = atmega16a)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If there was a bug fixed, then there should be a bug report/description somewhere, yes?

Where does avr-gcc code generation bugs go to get registered? To avrlibc? Or gcc?

Is pr39633 a "ticket number"?

(Taking this thread as an opportunity to learn something..)

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

@clawson: I don't manage to get your code (I see it in avr-gcc source); you are specifying other flags than -Os, e.g. -g or others. But even with -g or -gdwarf-2 I do't get you code.

Target: avr
Configured with: ../gcc-4.3.3/configure --enable-win32-registry=WinAVR-20100110 --with-gmp=/usr/local --with-mpfr=/usr/local --prefix=/c/WinAVR --target=avr --enable-languages=c,c++,objc --with-dwarf2 --enable-doc --disable-shared --disable-libada --disable-libssp --disable-nls --with-pkgversion='WinAVR 20100110' --with-bugurl='URL:http://sourceforge.net/tracker/?atid=520074&group_id=68108&func=browse'
Thread model: single
gcc version 4.3.3 (WinAVR 20100110) 
GNU C (WinAVR 20100110) version 4.3.3 (avr)
	compiled by GNU C version 3.4.5 (mingw-vista special r3), GMP version 4.2.3, MPFR version 2.4.1.

Compiler executable checksum: 61d68a374065d489330774d2533cbbdf

PR stands for "problem report". Append the PR number to GCC web page like so:
http://gcc.gnu.org/PR39633
To search for PRs goto
http://gcc.gnu.org/bugzilla -> Search

If a PR is on a patched GCC and the PR cannot be reproduced it's not likely it will be fixed.

avr-gcc is GCC and GCC deverlopers care for bugs in GCC bugzilla (at least I do). I don't think they are inclined to browse the web to find some reports on patched versions or PR nor properly reported, e.g. sources with external #includes like avr/io.h or lcd.h, with code like ... for extra obfuscation, not telling command line option, compiler version, etc.

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

you are specifying other flags than -Os, e.g. -g or others. But even with -g or -gdwarf-2 I do't get you code.

The command (issued by Studio 4) was:

avr-gcc  -mmcu=atmega16a -Wall -gdwarf-2 -std=gnu99 --save-temps       -DF_CPU=8000000UL -Os -funsigned-char -funsigned-bitfields -fpack-struct -fshort-enums -MD -MP -MT test.o -MF dep/test.o.d  -c  ../test.c

Apart from the added --save-temps that's pretty standard.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Jokester, you are using -funsigned-char.
With that option the bug might still occur, on different code, of course.

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Jokester, you are using -funsigned-char.

Both Mfile and AVR Studio pass this option by default so almost everyone using the avr-gcc compiler is using this option.

Cliff

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Both Mfile and AVR Studio pass this option by default so almost everyone using the avr-gcc compiler is using this option.

:idea: Both Mfile and AVR Studio pass this option by default so almost everyone using Mfile or AVR Studio is using this option.

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

so almost everyone using Mfile or AVR Studio is using this option.

Which is probably about 80-90% of all users.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sorry that I did not install AVR Studio just to
guess what options you might have set.
Just tell us :-)

avrfreaks does not support Opera. Profile inactive.