is __ELF_WORD_SIZE == 32 really should be here?

I think so. Thing is, this code is built twice, but indirectly; see linux_elf64.c and linux_elf32.c.

I think this should be yet another sv_XXX method.

ok, I’m confused a bit)
For i386 we does not define compat_linux32

Why? We're already calling this through this indirection (as sv_coredump); I don't see the point of adding more. Also, another ABI - if it ever happens - might need different tweaks here.

For me, your reply sounds as an additional strong argument pro change this to sv_XXX method. Exactly because this part is per-ABI.

The fact that it is called through sv_coredump does not matter, since you ended up in the shared code.

This isn't per-ABI - it's ABI-specific. In other words, one can expect each ABI to require this kind of quirk, but not this particular one, so we would be adding more and more sv_whatever variables to control each of them.

what is the purpose to ignore i386?

It doesn't build on i386, and I'm not particularly motivated to spending time on fixing a dead architecture, I have a live one that needs fixing ;-)

I think my main problem here is that adding another sv_variable makes things harder to understand (more indirection) and more boilerplate'y for no good reason - it won't make the code cleaner, shorter, or more flexible.

Of course it makes the code cleaner, and we might argue about flexibility. But not having the FreeBSD ABI coredump code to know/care about other ABIs is the good design by definition.

Also, I do not get the argument that some ABIs would need to do more specializations there. If the differences are large, ABI would provide more then one sysent classes, if not, it would handle it internally. But even more then, I do not want to contaminate the base FreeBSD code with the irrelevant details to handle foreign ABIs.

Okay, what do you think about this instead? It avoids having any Linux-specific code in the common path, is simple, flexible, and avoids introducing another sysent entry.

Now that there is no common code changed in your patch, I looked at the linux/ diff.

I would not call this code similar. This function is a literal copy of the FreeBSD native coredump, modulo the removal of compressed dump support.

And this function is again the exact copy of the FreeBSD' corehdr except the added assignment to e_ident.

I do not think this is a good approach. Returning to the previous suggestion of adding some sv_ methods, I think this is the way to go instead of copying 90% of coredumping code and making small modifications here and there.

If you do not like adding sv_ methods for ELF coredump, consider creating a table of virtual methods just for ELF coredump and passing it as an arg to coredump(). But this copying of ~300 LoC should be fixed IMO.

I thought there would be too many differences to do this properly, but turns out you were right - it's just two additional sv_ variables and one method, with pretty much no duplication left.

Can you change the interface to add either struct sysentvec *sv or struct thread *td as the argument, to avoid referencing curproc there?

I know that coredump code have more dependencies on the context so it cannot dump remote process, but I do not want to introduce new deps.

Same (please add td or sv arg)/

I am not sure that this static definition has the effect you intended, formally it add the instance of FREEBSD_ABI_VENDOR to all files including this header.

I suggest to name the fields in style of sv_elf_core_XXX

Extra ()

What are the differences between this code and FreeBSD native prpsinfo?

The cast is not needed.

Cast is not needed.
Perhaps you can fix these casts in FreeBSD code as the preliminary commit.

But this function is still the exact copy of the FreeBSD function?

Same code, again?

Is this line too long?

I think none at the moment, but there's a large chance they will diverge. This code is in a "good enough" state, but it'll require spending some more time comparing its output with what Linux kernel generates. Thus, I'd rather not deduplicate them just yet.

You might want to fix this in imgact_elf.c as a preliminary commit.

I suspect that prpsinfo goes back to SysV, so the format of the note is engraved in stone.

Why 320?

And similar question for the native note_procstat_auxv, where it might become too short quite fast (128).

I mean, it should be based on AT_COUNT and equivalent Linux constant.

Spun off as https://reviews.freebsd.org/D30841.

I'm not sure anything can be engraved in stone when we're talking about Linux :-) In any case: if it turns out these are same between FreeBSD and Linux, I'll deduplicate them later on.

BTW I think all entries can use elf64 instead of elfN(). This is for later, there are existing elfN() uses.

In fact, I would be more happy about leaving this functions redefined if you used the names like l_prstatus_t instead of using exact same name as the FreeBSD native code.

If it is still elf_prpsinfo_t, you do not need a copy of the function. Both prpsinfo and prfpregset should be linux-named IMO.

It is elf_prpsinfo_t _for now_; it will likely change later. I'd prefer to avoid a scenario where we add complexity, exporting the native function and using it here, only to find that we actually do need to do something differently here. Deduplicating stuff afterwards is easy.

Why did you left the generic name for this (and one below) types?

These should be e.g. linux_prpsinfo_t. Do not use elf_ prefix in linux code which you refuse to de-dup.

This cast is not needed.