Mikael Pettersson writes:
> This patch fixes two problems with the m68k-specific bitops
> in e2fsprogs.
>
> The first problem is that tst_bitops SEGVs:
>
> LD_LIBRARY_PATH=../../lib DYLD_LIBRARY_PATH=../../lib ./tst_bitops
> make[1]: *** [check] Segmentation fault
>
> Post-mortem debugging shows that this occurs in the m68k-specific
> ext2fs_set_bit() when called with BIG_TEST_BIT as bit number:
>
> Core was generated by `./tst_bitops'.
> Program terminated with signal 11, Segmentation fault.
> #0 0x80000820 in ext2fs_set_bit (addr=0xc01ba008, nr=2147483690) at ../../lib/ext2fs/bitops.h:357
> 357 __asm__ __volatile__ ("bfset %2@{%1:#1}; sne %0"
> (gdb) bt
> #0 0x80000820 in ext2fs_set_bit (addr=0xc01ba008, nr=2147483690) at ../../lib/ext2fs/bitops.h:357
> #1 main (argc=1, argv=0xefc31174) at tst_bitops.c:112
> (gdb) up
> #1 main (argc=1, argv=0xefc31174) at tst_bitops.c:112
> 112 ext2fs_set_bit(BIG_TEST_BIT, bigarray);
>
> The SEGV occurs because bfset et al interpret the bit number as a
> _signed_ value, so 2147483690 is actually -2147483606, causing an
> access to a location about 1/4 GB before 'bigarray'.
>
> To fix this I adjust the base address to point to the correct byte,
> and limit the bit number to be within that byte, before performing
> the bfset (and likewise in the clear_bit and test_bit functions).
> This is similar to what the x86-specific bitops do.
>
> With this in place the SEGV is gone and the tests pass, but there
> is still some strange output from tst_bitops:
>
> big bit number (2147483690) test: 0, expected 4
> big bit number (2147483690) test: 4, expected 0
> ext2fs_set_bit big_test successful
>
> The test and expected values differ, yet the test case doesn't fail.
>
> A second look at the m68k bitop asm() statements makes the reason clear:
> there are _no_ dependencies whatsoever between the asm()s and the
> memory locations being accessed (a dependency on the address isn't
> enough to create a dependency on memory locations reachable from it,
> and __volatile__ doesn't create memory dependencies either). The
> simplest fix is to add "memory" clobbers (the kernel's bitops also
> have memory clobbers), and with those in place the tests still pass
> and the debug output looks sane:
>
> big bit number (2147483690) test: 4, expected 4
> big bit number (2147483690) test: 0, expected 0
> ext2fs_set_bit big_test successful
>
> Tested on m68k-linux with e2fsprogs-1.42.6 and gcc-4.6.3.
Andreas Schwab suggested that there was no longer an advantage
to using inline asm for these operations, so this new patch
deletes the m68k-specific code from lib/ext2fs/bitops.h (leaving
only 32-bit x86 to still use inline asm there).
Tested on m68k-linux with e2fsprogs-1.42.6 and gcc-4.6.3 as before.
All tests pass and the debug output looks sane.
I compared the e2fsck binaries from the previous build with this
one. They had identical .text sizes, and almost the same number
of bit field instructions (obviously compiler-generated), so this
change should have no serious performance implications.
Signed-off-by: Mikael Pettersson <mikpe@xxxxxxxx>
Cc: Andreas Schwab <schwab@xxxxxxxxxxxxxx>
---
lib/ext2fs/bitops.h | 39 +--------------------------------------
1 files changed, 1 insertion(+), 38 deletions(-)
--- e2fsprogs-1.42.6/lib/ext2fs/bitops.h.~1~ 2012-09-10 04:53:00.000000000 +0200
+++ e2fsprogs-1.42.6/lib/ext2fs/bitops.h 2012-12-16 17:41:09.000000000 +0100
@@ -200,7 +200,7 @@ extern errcode_t ext2fs_find_first_zero_
*/
#ifdef NO_INLINE_FUNCS
#if (defined(__GNUC__) && (defined(__i386__) || defined(__i486__) || \
- defined(__i586__) || defined(__mc68000__)))
+ defined(__i586__)))
/* This prevents bitops.c from trying to include the C */
/* function version of these functions */
#define _EXT2_HAVE_ASM_BITOPS_
@@ -345,43 +345,6 @@ _INLINE_ __u16 ext2fs_swab16(__u16 val)
#endif /* i386 */
-#if ((defined __GNUC__) && !defined(_EXT2_USE_C_VERSIONS_) && \
- (defined(__mc68000__)))
-
-#define _EXT2_HAVE_ASM_BITOPS_
-
-_INLINE_ int ext2fs_set_bit(unsigned int nr,void * addr)
-{
- char retval;
-
- __asm__ __volatile__ ("bfset %2@{%1:#1}; sne %0"
- : "=d" (retval) : "d" (nr^7), "a" (addr));
-
- return retval;
-}
-
-_INLINE_ int ext2fs_clear_bit(unsigned int nr, void * addr)
-{
- char retval;
-
- __asm__ __volatile__ ("bfclr %2@{%1:#1}; sne %0"
- : "=d" (retval) : "d" (nr^7), "a" (addr));
-
- return retval;
-}
-
-_INLINE_ int ext2fs_test_bit(unsigned int nr, const void * addr)
-{
- char retval;
-
- __asm__ __volatile__ ("bftst %2@{%1:#1}; sne %0"
- : "=d" (retval) : "d" (nr^7), "a" (addr));
-
- return retval;
-}
-
-#endif /* __mc68000__ */
-
#if !defined(_EXT2_HAVE_ASM_SWAB_)
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
