padding is needed for 64-bit big endian
ClosedPublic

Authored by gniibe on Apr 7 2017, 1:07 AM.

Details

Summary

We have an issue of gpgscm on s390x.
The _flag field should be compatible to vector element for gc.
On the machine with 64-bit big endian, LSB of _flag is not the LSB of the word of vector element when we don't have this fix.

This is not yet perfect patch, it's just for testing.
This doesn't work for LLP64 system (we need long long there), only for LP64 or LP32.

For real use, we need to detect endian-ness and size of int and add padding.

Test Plan

On porterbox, I'll test if it works.

Diff Detail

Repository
rG GnuPG
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
gniibe updated this revision to Diff 1256.Apr 7 2017, 2:27 AM

Update of the diff, so that we can keep _flag field access to 32-bit on 64-bit machine.

gniibe added a comment.Apr 7 2017, 3:05 AM

Please decide for _flag access on 64-bit machine, if 32-bit access is better or not.
If 64-bit access is better, update version of {Diff1255} is needed, instead.
unsigned long is not good for LLP64 system. Use unsigned long long for 64-bit system.

@gniibe thanks for figuring this out. This is rather mundane, I'm somewhat astonished that I did not think of that :(.

Do you know why this is only sometimes causing problems? I have never once seen a crash on my s390 emulator, yet @dkg seems to have no problem reproducing it.

Wouldn't a nicer and simpler fix be to use a machine-word-sized integer for _flag? Do you see any downside to that?

I think that there are two archs: s390 and s390x. Latter is 64-bit and supports 32-bit version as well.
Use of machine word size (32-bit for 32-bit machine, 64-bit for 32-bit machine) is good. That will be update of Diff 1255.
But I don't know how it is achieved easily.
(If we can ignore LLP64, we can use unsigned long.)
Please go ahead that way.

In D421#2968, @gniibe wrote:

I think that there are two archs: s390 and s390x. Latter is 64-bit and supports 32-bit version as well.

I believe I have an emulated s390x:

$ uname -a
Linux s390 4.9.0-2-s390x #1 SMP Debian 4.9.13-1 (2017-02-27) s390x GNU/Linux

(gdb) print sizeof (void *)
$1 = 8
(gdb) print sizeof (int)
$2 = 4

Use of machine word size (32-bit for 32-bit machine, 64-bit for 32-bit machine) is good. That will be update of Diff 1255.
But I don't know how it is achieved easily.
(If we can ignore LLP64, we can use unsigned long.)

uintptr_t should do the trick.

I understand your emulator is s390x. Perhaps, on the emulator, memory layout is different?
I now see that C99 is OK for GnuPG, or at least no problem for gpgscm.

justus updated this revision to Diff 1257.Apr 7 2017, 12:53 PM
justus edited the test plan for this revision. (Show Details)

Use pointer-sized unsigned integer for the flags field.

This revision was automatically updated to reflect the committed changes.