mailing list of musl libc
 help / color / mirror / Atom feed
* [musl] Some pending changes/patches
@ 2021-01-17 19:42 Érico Nogueira
  2021-01-18  4:11 ` Érico Nogueira
  2021-01-30 21:40 ` Rich Felker
  0 siblings, 2 replies; 3+ messages in thread
From: Érico Nogueira @ 2021-01-17 19:42 UTC (permalink / raw)
  To: musl

Hi!

Firstly, congratulations on the new release :)

Secondly, I have some pending patches that I'd like to ping, plus some
changes that I was asked to post about once the 1.2.2 release had been
made.

- add cloexec flag to open() in pthread_setname_np:
https://www.openwall.com/lists/musl/2020/12/24/1

- (low-priority) add pthread_getname_np:
	- my version (needs to be cleaned up for the change in the patch
	  above):
	  https://www.openwall.com/lists/musl/2020/12/19/3
	  It tries to avoid duplicating the setup for the
	  /proc/self/task/%d/com path by adding a helper function.
	- older version:
	  https://inbox.vuxu.org/musl/AM0PR07MB442007DDD4FECFF99BB5EBF48BE70@AM0PR07MB4420.eurprd07.prod.outlook.com/
	  Lacks cloexec flag when opening file.

- (low priority) implement gethostid() beyond a stub:
https://inbox.vuxu.org/musl/20200804224230.27774-1-ericonr@disroot.org/

- fix type for __libc_start_main. In crt1.c and rcrt1.c, it is:

int __libc_start_main(int (*)(), int, char **,
	void (*)(), void(*)(), void(*)());

but in __libc_start_main.c, it is

int __libc_start_main(int (*main)(int,char **,char **), int argc, char **argv)

as far as I can tell the fix is simple and the signature mismatch isn't
an issue. Since I don't know the context for the mismatch and therefore
wouldn't be able to write an appropriate commit message, I refrained
from sending a patch for it.

Happy new year,
Érico

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [musl] Some pending changes/patches
  2021-01-17 19:42 [musl] Some pending changes/patches Érico Nogueira
@ 2021-01-18  4:11 ` Érico Nogueira
  2021-01-30 21:40 ` Rich Felker
  1 sibling, 0 replies; 3+ messages in thread
From: Érico Nogueira @ 2021-01-18  4:11 UTC (permalink / raw)
  To: musl

Sorry for the noise, I forgot one patch:

- use SYS_openat instead of SYS_open in open(2) to avoid spurious
fcntl syscalls:
https://inbox.vuxu.org/musl/20201229225518.3677-1-ericonr@disroot.org/

All the questions in the patch still apply, so I don't believe it's 
ready to be merged, but a review would be very helpful - pointers on the 
matter of the kernel timeline regarding SYS_openat and O_CLOEXEC flag 
would also be appreciated. And maybe it should be using __syscall_cp for 
the syscall (I believe this makes a difference for cancellation points)?

Cheers

Em 17/01/2021 16:42, Érico Nogueira escreveu:
> Hi!
> 
> Firstly, congratulations on the new release :)
> 
> Secondly, I have some pending patches that I'd like to ping, plus some
> changes that I was asked to post about once the 1.2.2 release had been
> made.
> 
> - add cloexec flag to open() in pthread_setname_np:
> https://www.openwall.com/lists/musl/2020/12/24/1
> 
> - (low-priority) add pthread_getname_np:
> 	- my version (needs to be cleaned up for the change in the patch
> 	  above):
> 	  https://www.openwall.com/lists/musl/2020/12/19/3
> 	  It tries to avoid duplicating the setup for the
> 	  /proc/self/task/%d/com path by adding a helper function.
> 	- older version:
> 	  https://inbox.vuxu.org/musl/AM0PR07MB442007DDD4FECFF99BB5EBF48BE70@AM0PR07MB4420.eurprd07.prod.outlook.com/
> 	  Lacks cloexec flag when opening file.
> 
> - (low priority) implement gethostid() beyond a stub:
> https://inbox.vuxu.org/musl/20200804224230.27774-1-ericonr@disroot.org/
> 
> - fix type for __libc_start_main. In crt1.c and rcrt1.c, it is:
> 
> int __libc_start_main(int (*)(), int, char **,
> 	void (*)(), void(*)(), void(*)());
> 
> but in __libc_start_main.c, it is
> 
> int __libc_start_main(int (*main)(int,char **,char **), int argc, char **argv)
> 
> as far as I can tell the fix is simple and the signature mismatch isn't
> an issue. Since I don't know the context for the mismatch and therefore
> wouldn't be able to write an appropriate commit message, I refrained
> from sending a patch for it.
> 
> Happy new year,
> Érico
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [musl] Some pending changes/patches
  2021-01-17 19:42 [musl] Some pending changes/patches Érico Nogueira
  2021-01-18  4:11 ` Érico Nogueira
@ 2021-01-30 21:40 ` Rich Felker
  1 sibling, 0 replies; 3+ messages in thread
From: Rich Felker @ 2021-01-30 21:40 UTC (permalink / raw)
  To: Érico Nogueira; +Cc: musl

On Sun, Jan 17, 2021 at 04:42:31PM -0300, Érico Nogueira wrote:
> - fix type for __libc_start_main. In crt1.c and rcrt1.c, it is:
> 
> int __libc_start_main(int (*)(), int, char **,
> 	void (*)(), void(*)(), void(*)());
> 
> but in __libc_start_main.c, it is
> 
> int __libc_start_main(int (*main)(int,char **,char **), int argc, char **argv)
> 
> as far as I can tell the fix is simple and the signature mismatch isn't
> an issue. Since I don't know the context for the mismatch and therefore
> wouldn't be able to write an appropriate commit message, I refrained
> from sending a patch for it.

I've wanted to fix this, but actually I don't know if we can; it needs
analysis. Defining the function with the unused junk args imposes an
ABI constraint that the caller (__libc_start_main) is entered with
argument space for 6 arguments. On some pass-by-register archs this is
not a constraint at all, but on pass-by-stack archs or archs where the
ABI requires the caller to reserve stack slots for the callee to spill
argument registers into, it does. If there's not sufficient space
reserved here, __libc_start_main could clobber space that overlaps
with argv[].

Now, crt1.c makes the call correctly with space reserved for 6
arguments. But prior to the switch to crt1.c and crt_arch.h, there was
separate per-arch asm making the call to __libc_start_main, and some
archs might have omitted the stack space for these slots. So this old
asm needs to be read to determine if there may be binaries calling the
function as if it were a 3-arg one.

I *think* we're okay here. The asm was removed in commit
6fef8cafbd0f6f185897bc87feb1ff66e2e204e1, and at that time (2015), all
the remaining asm versions seemed to still be passing the extra 3
args, despite __libc_start_main having dropped use of them much
earlier (2013, commit 7586360badcae6e73f04eb1b8189ce630281c4b2). But I
would like to review it further before making a change here.

Rich

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-30 21:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-17 19:42 [musl] Some pending changes/patches Érico Nogueira
2021-01-18  4:11 ` Érico Nogueira
2021-01-30 21:40 ` Rich Felker

mailing list of musl libc

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/musl

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 musl musl/ http://inbox.vuxu.org/musl \
		musl@inbox.vuxu.org
	public-inbox-index musl

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.musl


code repositories for the project(s) associated with this inbox:

	https://git.vuxu.org/mirror/musl/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git