Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Big Endian Detection: Add a whitelist of always BE OSs #45

Merged
merged 1 commit into from Aug 2, 2018

Conversation

avar
Copy link
Contributor

@avar avar commented Aug 1, 2018

Hopefully fix the issue with AIX falling through this detection logic
and being detected as Little Endian reported on the Git mailing list,
see
https://public-inbox.org/git/20180729200623.GF945730@genre.crustytoothpaste.net/

Attempt to solve this by extending the existing fallback we have for
detecting always Big Endian processors to also detecting always Big
Endian OSs. See the public-inbox link in the comment I'm adding for
why this should work.

I have not tested this myself, since I have no access to AIX. I'll be
pointing Michael Felt to the relevant MR so he can test it before this
is merged.

Hopefully fix the issue with AIX falling through this detection logic
and being detected as Little Endian reported on the Git mailing list,
see
https://public-inbox.org/git/20180729200623.GF945730@genre.crustytoothpaste.net/

Attempt to solve this by extending the existing fallback we have for
detecting always Big Endian processors to also detecting always Big
Endian OSs. See the public-inbox link in the comment I'm adding for
why this should work.

I have not tested this myself, since I have no access to AIX. I'll be
pointing Michael Felt to the relevant MR so he can test it before this
is merged.
@avar
Copy link
Contributor Author

avar commented Aug 1, 2018

As it turns out I've been able to test this myself on AIX (I got access to the GCC compile farm), and I can confirm that this works. On xlc 13.1.3 we take this branch, but on GCC 7.2.0 we take the earlier GCC-specific codepath.

I submitted this without knowing about #42 (even though I originally said they could submit a PR here, I forgot), looking at that PR I think this is probably a better approach, since I'm told that AIX is always BE, stuff that isn't AIX won't define _AIX, and it's useful to have this "is this OS always BE?" facility for future additions.

@MrShark @jrn @shumow @cr-marcstevens @kencu @aixtools: What do you think?

@aixtools
Copy link

aixtools commented Aug 2, 2018

I think your process is very gcc dependent and/or very gnu dependent.

root@x066:[/data/prj/aixtools/git/sha1test]make clean
find . -type f -name '.a' -print -delete
find: 0652-017 -delete is not a valid option.
make: [Makefile:102: clean] Error 1 (ignored)
find . -type f -name '
.d' -print -delete
find: 0652-017 -delete is not a valid option.
make: [Makefile:103: clean] Error 1 (ignored)
find . -type f -name '.o' -print -delete
find: 0652-017 -delete is not a valid option.
make: [Makefile:104: clean] Error 1 (ignored)
find . -type f -name '
.la' -print -delete
find: 0652-017 -delete is not a valid option.
make: [Makefile:105: clean] Error 1 (ignored)
find . -type f -name '.lo' -print -delete
find: 0652-017 -delete is not a valid option.
make: [Makefile:106: clean] Error 1 (ignored)
find . -type f -name '
.so' -print -delete
find: 0652-017 -delete is not a valid option.
make: [Makefile:107: clean] Error 1 (ignored)
find . -type d -name '.libs' -print | xargs rm -rv
rm -rf bin

mkdir -p bin && ar cru bin/libsha1detectcoll.a obj_lib/ubc_check.lo obj_lib/sha1.lo
cc obj_src/main.lo -Lbin -lsha1detectcoll -o bin/sha1dcsum
cc: 1501-218 (W) file obj_src/main.lo contains an incorrect file suffix
cc obj_src/main.lo -Lbin -lsha1detectcoll -o bin/sha1dcsum_partialcoll
cc: 1501-218 (W) file obj_src/main.lo contains an incorrect file suffix

Anyway, using xlc it also seems to PASS:

root@x066:[/data/prj/aixtools/git/sha1test]make test
test e98a60b463a6868a6ce351ab0166c0af0c8c4721 != bin/sha1dcsum test/sha1_reducedsha_coll.bin | cut -d' ' -f1 || (echo "\nError: Compiled for incorrect endianness" && false)
test a56374e1cf4c3746499bc7c0acb39498ad2ee185 = bin/sha1dcsum test/sha1_reducedsha_coll.bin | cut -d' ' -f1
test 16e96b70000dd1e7c85b8368ee197754400e58ec = bin/sha1dcsum test/shattered-1.pdf | cut -d' ' -f1
test e1761773e6a35916d99f891b77663e6405313587 = bin/sha1dcsum test/shattered-2.pdf | cut -d' ' -f1
test dd39885a2a5d8f59030b451e00cb45da9f9d3828 = bin/sha1dcsum_partialcoll test/sha1_reducedsha_coll.bin | cut -d' ' -f1
test d3a1d09969c3b57113fd17b23e01dd3de74a99bb = bin/sha1dcsum_partialcoll test/shattered-1.pdf | cut -d' ' -f1
test 92246b0b718f4c704d37bb025717cbc66babf102 = bin/sha1dcsum_partialcoll test/shattered-2.pdf | cut -d' ' -f1
bin/sha1dcsum test/*
a56374e1cf4c3746499bc7c0acb39498ad2ee185 test/sha1_reducedsha_coll.bin
16e96b70000dd1e7c85b8368ee197754400e58ec coll test/shattered-1.pdf
e1761773e6a35916d99f891b77663e6405313587 coll test/shattered-2.pdf
bin/sha1dcsum_partialcoll test/*
dd39885a2a5d8f59030b451e00cb45da9f9d3828 coll test/sha1_reducedsha_coll.bin
d3a1d09969c3b57113fd17b23e01dd3de74a99bb coll test/shattered-1.pdf
92246b0b718f4c704d37bb025717cbc66babf102 coll test/shattered-2.pdf

@avar
Copy link
Contributor Author

avar commented Aug 2, 2018

@aixtools I test this on AIX by building git with it, i.e. manually applying this patch to the sha1dc/sha1.c file, but the above also looks good!

@shumow shumow merged commit 232357e into cr-marcstevens:master Aug 2, 2018
@shumow
Copy link
Collaborator

shumow commented Aug 2, 2018

Thank you Ævar and Michael.

avar added a commit to avar/git that referenced this pull request Aug 2, 2018
Update sha1dc from the latest version by the upstream
maintainer[1]. See 2db8732 ("Merge branch 'ab/sha1dc'", 2017-07-10)
for the last update.

This fixes an issue where AIX was wrongly detected as a Little-endian
instead of a Big-endian system. See [2][3][4].

1. cr-marcstevens/sha1collisiondetection@232357e
2. cr-marcstevens/sha1collisiondetection#45
3. cr-marcstevens/sha1collisiondetection#42
4. https://public-inbox.org/git/20180729200623.GF945730@genre.crustytoothpaste.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
gitster pushed a commit to git/git that referenced this pull request Aug 2, 2018
Update sha1dc from the latest version by the upstream
maintainer[1]. See 2db8732 ("Merge branch 'ab/sha1dc'", 2017-07-10)
for the last update.

This fixes an issue where AIX was wrongly detected as a Little-endian
instead of a Big-endian system. See [2][3][4].

1. cr-marcstevens/sha1collisiondetection@232357e
2. cr-marcstevens/sha1collisiondetection#45
3. cr-marcstevens/sha1collisiondetection#42
4. https://public-inbox.org/git/20180729200623.GF945730@genre.crustytoothpaste.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants