nodejs / http-parser

http request/response parser for c
MIT License
6.32k stars 1.53k forks source link

armv7hl: Assertion `sizeof(http_parser) == 4 + 4 + 8 + 2 + 2 + 4 + sizeof(void *)' failed #507

Closed ignatenkobrain closed 4 years ago

ignatenkobrain commented 4 years ago

https://github.com/nodejs/http-parser/blob/7af127d3ac497bb036d3ecb4fb24bb5ee7ff4b5b/test.c#L4224

This does not work on armv7hl…

bnoordhuis commented 4 years ago

Can you check what sizeof(http_parser) is (and why)?

thefloweringash commented 4 years ago

The assertion is not applying padding rules. The difference between sizeof(http_parser)=32 and offsetof(http_parser, data)=24 shows a 4-byte pointer and 4-bytes of padding after it. I haven't found a succinct source, but a stack overflow post on Structure padding and packing asserts:

For struct, other than the alignment need for each individual member, the size of whole struct itself will be aligned to a size divisible by size of largest individual member, by padding at end.

Which is consistent with this struct being padded to a multiple of 8-bytes, since it contains an 8-byte uint64_t content_length.

./test_g
http_parser v2.9.4 (0x020904)
sizeof(http_parser) = 32
sizeof(void *) = 4
offsetof(http_parser, nread) = 4
offsetof(http_parser, content_length) = 8
offsetof(http_parser, http_major) = 16
offsetof(http_parser, http_minor) = 18
offsetof(http_parser, data) = 24
test_g: test.c:4230: main: Assertion `sizeof(http_parser) == 4 + 4 + 8 + 2 + 2 + 4 + sizeof(void *)' failed.
With a small patch to show some offsets ```diff diff -ur source/test.c source-debug/test.c --- source/test.c 1970-01-01 00:00:01.000000000 +0000 +++ source-debug/test.c 2020-04-29 18:31:08.678915685 +0000 @@ -4221,6 +4221,12 @@ printf("http_parser v%u.%u.%u (0x%06lx)\n", major, minor, patch, version); printf("sizeof(http_parser) = %u\n", (unsigned int)sizeof(http_parser)); + printf("sizeof(void *) = %u\n", (unsigned int)sizeof(void*)); + printf("offsetof(http_parser, nread) = %u\n", (unsigned int)offsetof(http_parser, nread)); + printf("offsetof(http_parser, content_length) = %u\n", (unsigned int)offsetof(http_parser, content_length)); + printf("offsetof(http_parser, http_major) = %u\n", (unsigned int)offsetof(http_parser, http_major)); + printf("offsetof(http_parser, http_minor) = %u\n", (unsigned int)offsetof(http_parser, http_minor)); + printf("offsetof(http_parser, data) = %u\n", (unsigned int)offsetof(http_parser, data)); assert(sizeof(http_parser) == 4 + 4 + 8 + 2 + 2 + 4 + sizeof(void *)); ```
bnoordhuis commented 4 years ago

That makes sense, thanks. #510 should fix it.

haudan commented 4 years ago

I can reproduce this on HP-UX (Itanium machine, ia64).

Using @thefloweringash's modified test.c:

http_parser v2.9.4 (0x020904)
sizeof(http_parser) = 32
sizeof(void *) = 4
offsetof(http_parser, nread) = 4
offsetof(http_parser, content_length) = 8
offsetof(http_parser, http_major) = 16
offsetof(http_parser, http_minor) = 18
offsetof(http_parser, data) = 24
Assertion failed: sizeof(http_parser) == 4 + 4 + 8 + 2 + 2 + 4 + sizeof(void *), file test_g.c, line 4309
Abort(coredump)
bnoordhuis commented 4 years ago

I fixed this in 4f15b7d by checking the struct size only for i386 and x86_64, that should hopefully be good enough to catch regressions.