exercism / x86-64-assembly

Exercism exercises in x86-64 Assembly.
https://exercism.org/tracks/x86-64-assembly
MIT License
22 stars 18 forks source link

Quickfix for failing tests of exercise resistor-color #176

Closed rainij closed 1 year ago

rainij commented 1 year ago

Currently the tests for resitor-color fail. I fixed that.

The error message (see one of the latest CI runs) is the following:

OK
/usr/bin/ld: resistor_color.o: warning: relocation in read-only section `.rodata'
/usr/bin/ld: warning: creating DT_TEXTREL in a PIE
collect2: error: ld returned 1 exit status
make: *** [Makefile:32: tests] Error 1
Error: Process completed with exit code 2.

I must admit that I do not 100% understand what is going on but putting the color_array into the .data section silenced the warning and made the tests green again. I am not an expert in this low-level stuff.

rdipardo commented 1 year ago

@rainij,

I must admit that I do not 100% understand what is going on but putting the color_array into the .data section silenced the warning and made the tests green again. I am not an expert in this low-level stuff.

Data in the .rodata section is "static", which literally means that each datum's memory address is fixed in place.

When you execute the lea instruction, you load the effective address of the source operand, similar to storing a pointer in a variable of char * type in C. Imagine you had #defined an array somewhere in a C source file, and you tried to obtain a pointer to it. Writing through that pointer will obviously cause a segmentation fault. The linker is just enforcing some common-sense memory safety.

rainij commented 1 year ago

@rdipardo thanks for you reply. Yes I figured something along these lines already. That was also the reason I tried to insert .data section. Actually after solving the exercise I was just curious how the "official" solution looks like and it already looked odd to me that this should "assemble". It was only then when I noticed that the CI actually fails (so I had the fix before the I noticed the bug :smile:).

But I was suprised by some things. One of these things was the error message. The read-only part sounded reasonable to me. On the other hand the PIE part motivated me to try removing the -pie flag from the Makefile. This also "assembles" (or "links"). Of course I tried to look up what this is about and why the (same) linker is suddenly fine. But I didn't completely understand the relation so I postponed understanding this.

But my remark should just be taken as a warning that somebody with deeper knowledge should look over that. You seem to agree that the fix is correct. So we are already two - thanks :smile:.

bergjohan commented 1 year ago

Moving color_array into the .data section seems like the correct way to handle this.

I found a PR in the ocaml repo that has a good explanation: https://github.com/ocaml/ocaml/pull/11042

TL;DR

for shared objects or position-independent executables, read-only data containing pointers to symbols need relocation at load time and should better be put in a data segment.

bergjohan commented 1 year ago

Merged, thank you!

sudhackar commented 1 year ago

Just FYI I raised this back when the revamp was happening https://github.com/exercism/x86-64-assembly/issues/171

bergjohan commented 1 year ago

Ah, I'm sorry, I must have missed that because the bot automatically closed it, that's unfortunate.. :(