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

Works only on Little Endian #58

Open
BenWiederhake opened this issue Oct 13, 2015 · 15 comments
Open

Works only on Little Endian #58

BenWiederhake opened this issue Oct 13, 2015 · 15 comments

Comments

@BenWiederhake
Copy link
Contributor

Originally reported at majn/telegram-purple#98

All earlier discussion see there.

@BenWiederhake
Copy link
Contributor Author

I don't get it. read_types reads garbage for some values and then reads something correct again.

@mcepi: Can you upload the file auto/scheme.tlo somewhere? That file is platform-dependent, so one can't just copy it from machine to machine, but thankfulyl the format is simple enough to inspect it with a hex-editor.

For the record: The shortest way to reproduce this bug is ./configure && make auto/auto-skip.h (argument optional, but forces make to build only the failing target).

For reference, the output of generate should start like this (on my branch tmp/debug-tgp-98):

Found 157 types
read_types: name=1885708031, id=#, print_id=#, custructors_num=0
read_types: name=3100684255, id=AccountDaysTTL, print_id=account_days_t_t_l, custructors_num=1

However, for the reporter it starts like this (first line not available yet, he ran the version without that line):

read_types: name=1885708031, id=, print_id=, custructors_num=0
read_types: name=3100684255, id=, print_id=, custructors_num=1869966964

@vysheng
Copy link
Owner

vysheng commented Oct 14, 2015

Maybe he has big-endian platform?

@vysheng
Copy link
Owner

vysheng commented Oct 14, 2015

Looks like s390 is big-endian. Probably there are many problems with big-endian platforms.

@mcepl
Copy link

mcepl commented Oct 14, 2015

That is my experience ... of course, nobody sane runs libpurple-driven anything on s390, but building it there shows a ton of crazy bugs, which would otherwise were ignored (not mentioning that it is 31-bit architecture).

@BenWiederhake
Copy link
Contributor Author

1869966964 in 0x6F756E74 in hex, or ount in ASCII, which suspiciously looks like the "ount" in either "AccountDaysTTL" or "account_days_t_t_l".

@BenWiederhake
Copy link
Contributor Author

@vysheng: Why? tl-parser does a lot of parsing, and shouldn't care about the architecture it's running on.

@vysheng
Copy link
Owner

vysheng commented Oct 14, 2015

@BenWiederhake In tl-parser there is only one place (as I see now). It is serializing data. (And unserializing in generate). It's rather easy to fix. In tgl there are at least places that serialize/deserialize data from/to sockets. It's also not too hard to fix.

But I'm not sure, that I don't use pointer casts, that are invalid in big-endian platforms. Like casting (long long *) to (int *). It needs to be checked.

@mcepl
Copy link

mcepl commented Oct 16, 2015

Is there any change (it is 481689e now) in https://gist.github.com/mcepl/bd8585ca30cbfda6adff ?

@BenWiederhake
Copy link
Contributor Author

How do you mean? Why do you expect a difference? What changed? The latest revision of your gist still shows the same error, with the same message

@mcepl
Copy link

mcepl commented Oct 16, 2015

I was not sure whether it was the same commit. Sorry for bothering.

@BenWiederhake
Copy link
Contributor Author

@mcepi: No problem XD

Also, can you upload the file auto/scheme.tlo somewhere? I asked you before, but it seems my request got drowned out.

@BenWiederhake
Copy link
Contributor Author

Aargh, I was looking at get_int and wint all the time, assuming that get_string would use it to read/write the length. I mean, get_int is used everywhere ... except in the one place where it makes sense most. Instead:

  int l = *(unsigned char *)buf_ptr;

Vitaly, I am really thankful for libtgl, but this was a bad idea. It's the textbook example of what not to do. Literally!

I'll change the .tlo format (there's no compatibility and no need for it, so changing it should be okay), because get_int already is endian-correct. I'll look out for more issues like this.

@vysheng
Copy link
Owner

vysheng commented Oct 16, 2015

@BenWiederhake

You can change tlo format. But TL uses almost the same serialization and there you'll need to make it compatible.

@BenWiederhake
Copy link
Contributor Author

It's rather easy to fix tl-parser and generate, which was pleasantly easy: vysheng/tl-parser#5

However, it will be very troublesome to fix the main {en,de}coding, which assumes Little Endian everywhere. Here's an incomplete attempt that doesn't build but is hopefully at least on the right track.

@mcepl
Copy link

mcepl commented Oct 17, 2015

Here https://mcepl.fedorapeople.org/tmp/tgl.tar.gz is the whole build tree.

@BenWiederhake BenWiederhake changed the title FTBFS of 20150525 version on s390 Works only on Little Endian Nov 29, 2015
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

No branches or pull requests

3 participants