[PATCH] MIPS: Remove unnecessary zero-length struct member

Thorsten Blum posted 1 patch 10 months ago
arch/mips/include/asm/ptrace.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH] MIPS: Remove unnecessary zero-length struct member
Posted by Thorsten Blum 10 months ago
Remove the zero-length struct member '__last' and use sizeof() to
calculate the value for MAX_REG_OFFSET.

No functional changes intended.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 arch/mips/include/asm/ptrace.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/ptrace.h b/arch/mips/include/asm/ptrace.h
index 85fa9962266a..1bb10ee8c2ce 100644
--- a/arch/mips/include/asm/ptrace.h
+++ b/arch/mips/include/asm/ptrace.h
@@ -48,7 +48,6 @@ struct pt_regs {
 	unsigned long long mpl[6];        /* MTM{0-5} */
 	unsigned long long mtp[6];        /* MTP{0-5} */
 #endif
-	unsigned long __last[0];
 } __aligned(8);
 
 static inline unsigned long kernel_stack_pointer(struct pt_regs *regs)
@@ -65,7 +64,7 @@ static inline void instruction_pointer_set(struct pt_regs *regs,
 
 /* Query offset/name of register from its name/offset */
 extern int regs_query_register_offset(const char *name);
-#define MAX_REG_OFFSET (offsetof(struct pt_regs, __last))
+#define MAX_REG_OFFSET (sizeof(struct pt_regs))
 
 /**
  * regs_get_register() - get register value from its offset
-- 
2.49.0
Re: [PATCH] MIPS: Remove unnecessary zero-length struct member
Posted by Maciej W. Rozycki 9 months, 4 weeks ago
On Fri, 11 Apr 2025, Thorsten Blum wrote:

> Remove the zero-length struct member '__last' and use sizeof() to
> calculate the value for MAX_REG_OFFSET.
> 
> No functional changes intended.

 Have you verified that there's no change except for timestamp data in 
(non-debug) `vmlinux' produced with and w/o the patch applied?

 Also this is broken anyway: if you use MAX_REG_OFFSET for `offset' passed 
to `regs_get_register', then data past the end of `regs' will be accessed.

  Maciej
Re: [PATCH] MIPS: Remove unnecessary zero-length struct member
Posted by Thorsten Blum 9 months, 3 weeks ago
On 16. Apr 2025, at 04:33, Maciej W. Rozycki wrote:
> On Fri, 11 Apr 2025, Thorsten Blum wrote:
> 
>> Remove the zero-length struct member '__last' and use sizeof() to
>> calculate the value for MAX_REG_OFFSET.
>> 
>> No functional changes intended.
> 
> Have you verified that there's no change except for timestamp data in 
> (non-debug) `vmlinux' produced with and w/o the patch applied?
> 
> Also this is broken anyway: if you use MAX_REG_OFFSET for `offset' passed 
> to `regs_get_register', then data past the end of `regs' will be accessed.

Yes, true. It seems like

	if (unlikely(offset >= MAX_REG_OFFSET))
		return 0;

should do the trick.

The comment also says "If @offset is bigger than MAX_REG_OFFSET", rather
than "is bigger than or equal to".

Happy to add it to v2 or a separate patch, if this is actually correct?!

Thanks,
Thorsten
Re: [PATCH] MIPS: Remove unnecessary zero-length struct member
Posted by Maciej W. Rozycki 9 months, 3 weeks ago
On Thu, 17 Apr 2025, Thorsten Blum wrote:

> > Also this is broken anyway: if you use MAX_REG_OFFSET for `offset' passed 
> > to `regs_get_register', then data past the end of `regs' will be accessed.
> 
> Yes, true. It seems like
> 
> 	if (unlikely(offset >= MAX_REG_OFFSET))
> 		return 0;
> 
> should do the trick.

 No, other platforms use the same (offset > MAX_REG_OFFSET) check that we 
do and just set MAX_REG_OFFSET correctly to point at the last register in 
`struct pt_regs'.

> The comment also says "If @offset is bigger than MAX_REG_OFFSET", rather
> than "is bigger than or equal to".

 And quite correctly so.

> Happy to add it to v2 or a separate patch, if this is actually correct?!

 Yes, please send v2.  There's no point in changing MAX_REG_OFFSET twice.

  Maciej
Re: [PATCH] MIPS: Remove unnecessary zero-length struct member
Posted by Thorsten Blum 9 months, 3 weeks ago
On 16. Apr 2025, at 04:33, Maciej W. Rozycki wrote:
> On Fri, 11 Apr 2025, Thorsten Blum wrote:
> 
>> Remove the zero-length struct member '__last' and use sizeof() to
>> calculate the value for MAX_REG_OFFSET.
>> 
>> No functional changes intended.
> 
> Have you verified that there's no change except for timestamp data in 
> (non-debug) `vmlinux' produced with and w/o the patch applied?

Just did - and yes, the binaries are identical.

Thanks,
Thorsten