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

42 Subject change #2

Closed
aristotelis-bobas opened this issue Apr 30, 2020 · 11 comments
Closed

42 Subject change #2

aristotelis-bobas opened this issue Apr 30, 2020 · 11 comments

Comments

@aristotelis-bobas
Copy link

aristotelis-bobas commented Apr 30, 2020

The subject has changed on this project: https://cdn.intra.42.fr/pdf/pdf/10535/en.subject.pdf
We have to set the right errno after syscall errors.
For this we have to use extern ___error.

I have implemented this in the following way for ft_write (same error handling in ft_read):

_ft_write:                                  ; rdi = file descriptor, rsi = string, rdx = byte count
            mov         rax, 0x2000004      ; BSD / MacOS calling convention unlike Linux
            syscall
            jc          error               ; error sets carry flag, rax = errno
            ret
error:
            mov         r15, rax            ; save errno
            call        ___error            ; retrieve address to errno
            mov         [rax], r15          ; put errno in return value of __error (pointer to errno)
            mov         rax, -1
            ret

This works and sets the right errno.
But your test now return segfaults on ft_write.s and ft_read.s
I think it has to do with the pipe and accessing the external variable errno.
I tried the input from your tests without the pipe and there is no segfault.

While my old code with a simple error return passes all your tests (but does not set the right errno):

_ft_write:                                  ; rdi = file descriptor, rsi = string, rdx = byte count
            mov         rax, 0x2000004      ; BSD / MacOS calling convention unlike Linux
            syscall
            jc          error               ; error sets carry flag
            ret
error:
            mov         rax, -1
            ret

For evaluations on 42 campuses your test is very useful so I wanted to address this change in subject and the false (I'm not sure if this is the case?) segfaults it may show (on ft_read and ft_write).
Any thoughts on implementation of the errno checking and/or the segfault error?
Thank you for your time.

@cacharle
Copy link
Owner

cacharle commented May 1, 2020

Is the ___error function specific to macos? I'm on Linux and I couldn't find a man page on my system or online.
If ___error return the address of errno (?) why can't we just access errno directly?

As for your implementation, I think you have to return -1 if the buffer is NULL.

Personally I'm using a dirty trick where I'm calling the fcntl syscall on fd before making the write syscall, I assumed that the syscalls set errno but apparently it isn't the case.

Do you think the tests should check if errno is correct? For now it just checks that the function returns -1.
The issue is that there is a lot of different error to check and I don't understand half of them.

Your test output would be much appreciated.
Thanks for keeping me updated.

@cacharle
Copy link
Owner

cacharle commented May 1, 2020

I've made a new commit (86ee1c1) on the errno branch to show how we could test errno value for ft_write.

Unfortunatly I'm on Linux and I can't link the test with libasm.a.
So tell me if my changes broke everything or modify the test yourself since it's hard for me to fix things without feedback.

@aristotelis-bobas
Copy link
Author

aristotelis-bobas commented May 1, 2020

Hey there, thanks for your reply.

Documentation on ___error is really difficult to find.

Especially because on Linux, if a syscall fails, errno is set automatically and the returned value from the syscall will be a negative number. No need to set errno yourself using ___error.

On FreeBSD (MacOS at 42 Network) syscall errors will not set errno. Instead the syscall returns errno itself (positive number) and set the 'carry flag'. As you will have to return -1 from ft_write or ft_read when an error is encountered, you will need to access the external variable errno (within the errno.h library) and set it accordingly.

See the following links for more explanation.

This one helped me a lot: https://stackoverflow.com/questions/15304829/how-to-return-errno-in-assembly

The FreeBSD errno.h library that is mentioned in the Stack Overflow link: https://github.com/freebsd/freebsd/blob/master/sys/sys/errno.h

From the FreeBSD errno.h library in the link above:

int *	   __error(void);
#define	errno		(* __error())

So for your questions

Is the __error function specific to macos? I'm on Linux and I couldn't find a man page on my system or online.

I think so, when I access Linux man page for errno.h it says:

On some ancient systems, <errno.h> was not present or did not declare
errno, so that it was necessary to declare errno manually (i.e.,
extern int errno). Do not do this. It long ago ceased to be neces‐
sary, and it will cause problems with modern versions of the C
library.


If ___error return the address of errno (?) why can't we just access errno directly?

The address to errno is returned from ___error. But you have to actually set what's in that address to the right errno (which you get from syscall that goes wrong).
So if your fd is wrong, sycall of write will return 9 = (Bad file descriptor) and you will have to manually put 9 into the address of errno (that you got from ___error).

See my code and comments for an example.
When I syscall with rdi (file descriptor) = -1, rax (return value) from syscall will be 9 and carry flag is set.
So jump condition 'JC' (= jump if carry flag is set) will be met.
Rax (=9) will be saved in register R15.
I call ___error. Rax is now address to external variable errno.
I put R15 (errno = 9) manually into external variable errno (address to errno).
After that it returns -1.

_ft_write:                                  ; rdi = file descriptor, rsi = string, rdx = byte count
            mov         rax, 0x2000004      ; BSD / MacOS calling convention unlike Linux
            syscall
            jc          error               ; error sets carry flag, rax = errno
            ret
error:
            mov         r15, rax            ; save errno
            call        ___error            ; retrieve address to errno
            mov         [rax], r15          ; put errno in return value of __error (pointer to errno)
            mov         rax, -1
            ret

As for your implementation, I think you have to return -1 if the buffer is NULL.

If buffer == NULL, when I syscall the same applies as I wrote above.
Syscall will error, thus returning the right errno number (In this case rax = 14 = Bad Address).
Carry flag is set, so it will jump to the error part. Same as before, retrieving address to external variable errno and setting it accordingly. After that it returns -1.

The problem with checking RDI, RSI and RDX before doing the syscall and returning -1 is that you will not set the errno accordingly. Only the return value is correct (-1).

The way I check it is like this (not automatic, just reading standard output):

    a = write(FOPEN_MAX + 1, "abcdefghijklmnopqrstuvwxyz\n", 27);
	perror("write errno");
	errno = 100;
    b = ft_write(FOPEN_MAX + 1, "abcdefghijklmnopqrstuvwxyz\n", 27);
	perror("ft_write errno");
    printf("return write = %d\nreturn ft_write = %d\n", a, b);

I manually set errno = 100 between calls of write and ft_write because if you don't reset errno and your ft_write does not touch errno at all, you will just print the errno from the original write function.

--- I just looked at your new branch, I see you did a test that's similar including the reset. Will come back to you with the results. Thanks for your time again.

@cacharle
Copy link
Owner

cacharle commented May 1, 2020

Thanks for the detailed answer.

For the ___error function on Linux, I've looked at my errno.h header and it's the same function with a different name __errno_location.

I need to do the same errno checking for ft_read, the other functions use malloc but I have no idea how to simulate an ENOMEM error.

As for the pipe related thing, the EAGAIN looks suspicious:

EAGAIN The  file  descriptor  fd refers to a file other than a socket and has been marked nonblocking
       (O_NONBLOCK), and the write would block.  See open(2) for further details  on  the  O_NONBLOCK flag.

Since I set the read end of the pipe to non-blocking. (The test doesn't work with empty string if I don't).

@aristotelis-bobas
Copy link
Author

aristotelis-bobas commented May 1, 2020

Since I set the read end of the pipe to non-blocking. (The test doesn't work with empty string if I don't).

I was struggling with this in your new branch.
Didn't set any errno on my own test with empty string.
But errno was set to 35 in your test.
How did you fix this? Would like to try.

Screenshot from 2020-05-01 16-04-57

I have commented out the segfault tests on read and write because they fail.
I believe it has got to do with the pipe but I don't know exactly what's going wrong.
Is it my code? Or can it be something with fork / pipe in your test?

Here a screenshot of segfault tests exclusively.
Screenshot from 2020-05-01 16-10-37

Was thinking about malloc aswell, I don't know if it's technically a syscall.
Subject says:

• You must check for errors during syscalls and properly set them when needed
• Your code must set the variable errno properly.
• For that, you are allowed to call the extern ___error

And like you said, no easy / safe way of simulating an error on malloc.
Will try to find something to see if have to set errno manually when calling malloc aswell.

@cacharle
Copy link
Owner

cacharle commented May 1, 2020

malloc isn't a syscall but it calls mmap or brk which are. Since it's a function in libc it should set errno.

I have no idea why you would segfault, file descriptor are unique for each process so the created pipe should be duplicated when forking. (I had no problem with it and I don't see why your implementation should have any either)

But errno was set to 35 in your test.
How did you fix this? Would like to try.

I may have fucked up and declared my variable to check errno as an unsigned.

fixed in dfdb617

cacharle added a commit that referenced this issue May 4, 2020
prettier compatible with python < 3.6
removed unaccurate test for ft_read
@cacharle cacharle closed this as completed May 4, 2020
@yorgsone
Copy link

I saw couple of implementations for ft_write and I think yours is the best. I think the segfault comes from misalignment of the stack due to the call, this fixed it for me:
sub rsp, 8
call ___error
add rsp, 8
https://github.com/cirosantilli/x86-assembly-cheat/blob/master/x86-64/calling-convention.md

@cacharle
Copy link
Owner

@yorgsone Never heard of stack alignment in the calling convensions (tbh I think I just copy pasted the solution from https://stackoverflow.com/c/42network/questions/1494)

If you have a better explaination for people who encounter this issue, feel free to put it in the Common Issues section of the README.

@Zerrino
Copy link

Zerrino commented Apr 23, 2024

That's sorcelery right there, you need to sub rsp, 8 to call ___error ?? what the actual fck ???

@cacharle
Copy link
Owner

@Zerrino It's been too long, I don't remember the specifics. Maybe @yorgsone can enlighten you.

@alurm
Copy link

alurm commented Jun 20, 2024

Commenting since nobody seems to have mentioned this.

If ___error return the address of errno, why can't we just access errno directly?

In the C standard, errno is guaranteed to be thread local.

Paraphrasing the draft of the C11 standard (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1548.pdf), chapter 7.5:

[...] errno expands to a modifiable lvalue that has type int and thread local storage duration, the value of which is set to a positive error number by several library functions.

Here's an example implementation of __errno_location: https://git.musl-libc.org/cgit/musl/tree/src/errno/__errno_location.c

int *__errno_location(void) {
return &__pthread_self()->errno_val;
}

Here's information from the Linux manual: https://www.man7.org/linux/man-pages/man3/errno.3.html

errno is defined by the ISO C standard to be a modifiable lvalue of type int, and must not be explicitly declared; errno may be a macro. errno is thread-local; setting it in one thread does not affect its value in any other thread.

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

5 participants