[openssl-dev] FW: [openssl.org #3629] Bug report: "run" in speed.c should be declared as volatile

[Lawrence via RT]

One correction, the value of "run" can be changed asynchronously because of
the interaction of signal and alarm, alarm raise the signal, signal register
the handler, the handler is sig_done, 

From: Lawrence [mailto:lawrence at codeaurora.org] 
Sent: Wednesday, December 10, 2014 6:32 PM
To: 'rt at openssl.org'
Subject: Bug report: "run" in speed.c should be declared as volatile

 

Hello.

 

I am one of the llvm developer, I run into a situation that I think it is
bug in souce code: "run" in speed.c should be declared as volatile because
"run"'s value can be predicted.

 

The problem is the following code:

 

signal(SIGALRM,sig_done);

.

                for (j=0; j<SIZE_NUM; j++)

                        {

 
print_message(names[D_SHA256],c[D_SHA256][j],lengths[j]);

                        Time_F(START);

                        for (count=0,run=1; COND(c[D_SHA256][j]); count++)
// COND(c[D_SHA256][j]) == (run && count<0x7fffffff)

                                SHA256(buf,lengths[j],sha256);

                        d=Time_F(STOP);

                        print_result(D_SHA256,j,count,d);

                        }

 

 

At least on arm, signal doesn't block the program, so it continue to run
after that, during the executing of that loop (time is random), signal
handling function sig_done is called, run is set to 0, so the loop will be
terminated (usually in 3s), otherwise, it will take a long time to run.

 

When LTO comes along, compiler has knowledge about SHA256(), since it
doesn't change "run", so compiler determine that the value of "run" won't
change, as a optimization, compiler removed the load and checking of "run",
that make the loop take very long time to run.

 

My point is, even though the unexpected behavior is caused compiler
optimization, compiler didn't do anything wrong, because it doesn't know the
value of "run" could change asynchronously.

 

I think "run" should be declared as volatile int, because volatile exactly
mean it's value could change asynchronously.

 

I am not an expert on signaling handling, but that what I have seen under
gdb on arm, so please let me know if I am wrong.

 

Regards.

 

Lawrence Hu