Skip to content

[WRAPPER] Fix FTSENT layout mismatch - #4329

Open
lz-bro wants to merge 1 commit into
ptitSeb:mainfrom
lz-bro:fix-ftsent-layout
Open

[WRAPPER] Fix FTSENT layout mismatch#4329
lz-bro wants to merge 1 commit into
ptitSeb:mainfrom
lz-bro:fix-ftsent-layout

Conversation

@lz-bro

@lz-bro lz-bro commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Translate native FTSENT structures to x86-64 layout in fts_read wrapper. Handle nlink_t size difference (4 vs 8 bytes), stat structure translation, and linked-list pointer resolution via per-FTS mapping table.

@lz-bro
lz-bro force-pushed the fix-ftsent-layout branch from 4d60f97 to 0f37c91 Compare August 31, 2026 07:07
@ptitSeb
ptitSeb requested a review from ksco August 31, 2026 07:58

@ksco ksco left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR title says RISC-V but the changes are not restricted to RISC-V. How about Arm64, LoongArch and PPC64LE?

Comment thread src/include/myalign.h Outdated
unsigned long long int u_debugreg [8];
} my_x64_user_t;

struct x64_ftsent { /* x86_64 riscv64 */

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Paste related kernel/libc source links to prove this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it should not restricted to RISC-V.

#ifdef __x86_64__
# define __NLINK_T_TYPE		__SYSCALL_ULONG_TYPE
# define __FSWORD_T_TYPE	__SYSCALL_SLONG_TYPE
#else
# define __NLINK_T_TYPE		__UWORD_TYPE
# define __FSWORD_T_TYPE	__SWORD_TYPE
#endif

FTSENT define: https://github.com/gnutools/glibc/blob/c3a3a9808ad3ab4a3336836833f83288b672ccbf/io/fts.h

__NLINK_T_TYPE define: https://github.com/gnutools/glibc/blob/c3a3a9808ad3ab4a3336836833f83288b672ccbf/sysdeps/unix/sysv/linux/x86/bits/typesizes.h

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant that you should add related links into the code as comments.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@lz-bro
lz-bro force-pushed the fix-ftsent-layout branch from 0f37c91 to 2f06a9c Compare August 31, 2026 09:30
@lz-bro lz-bro changed the title Fix FTSENT layout mismatch between RISC-V and x86-64 [WRAPPER] Fix FTSENT layout mismatch Aug 31, 2026
@lz-bro
lz-bro force-pushed the fix-ftsent-layout branch 2 times, most recently from ce41794 to 91d1170 Compare September 1, 2026 01:16
Comment thread src/include/myalign.h Outdated
uint16_t fts_flags; /* 100 */ /* 96 */
uint16_t fts_instr; /* 102 */ /* 98 */
uint32_t _pad1; /* 104 */ /* --- */
struct x64_stat64 *fts_statp; /* 112 */ /* 104 */

@ksco ksco Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_pad1 is 104, but suddenly fts_statp becomes 112?

Bad math, sorry.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, where does the _pad1 come from? I looked into the link you pasted in the comments, but I didn't find _pad1.

Comment thread src/wrapped/wrappedlibc_private.h Outdated
GOM(fts_open, pFEpip)
GO(fts_read, pFp)
GOM(fts_read, pFEp)
GO(fts_set, iFppi)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that we don't need to wrap other fts functions?

Comment thread src/libtools/myalign.c Outdated
}
if (!node) {
node = malloc(sizeof(struct fts_node));
if (!node) return ENOMEM;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deadlock

Comment thread src/libtools/myalign.c Outdated
fts_list = node;
}
struct ftsent_node* n = malloc(sizeof(struct ftsent_node));
if (!n) return ENOMEM;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deadlock.

Comment thread src/libtools/myalign.c
pthread_mutex_unlock(&fts_mutex);
}

void AlignFTSENT(void* dest, void* source)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is never used.

@ptitSeb

ptitSeb commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Question: instead of using a hash-table with a lock, wich works but add a lot of complexity, why not using an in-place "align / unalign" scheme, like with box32 inplace shrink/unshrink, as the structure seems to have the same memory footprint on x64 & native architecture, that should be possible, and would be more efficient and simpler?

Translate native FTSENT structures to x86-64 layout in fts_read wrapper.
Handle nlink_t size difference (4 vs 8 bytes), stat structure translation,
and linked-list pointer resolution via per-FTS mapping table.
@lz-bro

lz-bro commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

But this will disrupt the internal state of the structure? I see that functions like fts_children will access sp->fts_cur and read the FTSENT layout (https://github.com/gnutools/glibc/blob/master/io/fts-common.c#L1206)

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

Successfully merging this pull request may close these issues.

3 participants