On 08/01/2018 11:37, Jonathan Wakely wrote:
> On 8 January 2018 at 09:13, Andrew Haley wrote:
>> On 07/01/18 21:27, Christer Solskogen wrote:
>>> Fixed!
>>>
>>> Wanna know something funny?
>>> Having a printf fixed the segfault. Without it, it would crash.
>>> So the workaround in read_kickstart (memory.cpp) was to add
>>> "volatile int i" instead of "int i".
>>>
>>> Just one simple printf("Value of i: %d\n", i); was enough to not segfault.
>>
>> That's probably not the real fix, unless you have another thread accessing
>> i.
>
> And even if another thread is accessing that variable, making it
> volatile does not make the code correct.
>
> It's possible that the bug is now hidden by the combination of
> volatile and a sequence point introduced by calling printf, but it's
> still a bug to access a shared variable without atomic ops, see
> http://isvolatileusefulwiththreads.com/
'i' is an automatic variable, no concurrent access involved.
This is the code under discussion:
https://github.com/midwan/amiberry/blob/dev/src/memory.cpp#L568
And the author cargo culted this a few hours ago:
https://github.com/midwan/amiberry/commit/0a21fb4625197d48871ebcf23487a589063710d9
Regards.
