search

explosion / cython-blis

💥 Fast matrix-multiplication as a self-contained Python library – no system dependencies!
Other
219 stars 37 forks source link

Clang UBSAN runtime warnings: UndefinedBehaviorSanitizer: invalid-shift-base (linux-x86_64) #58

Closed patricksnape closed 3 years ago

patricksnape commented 3 years ago

When trying to build blis with my local compiler setup (linux-x86_64 target) - clang complains about 

SUMMARY: UndefinedBehaviorSanitizer: blis.h

blis.h runtime error: left shift of 7 by 29 places cannot be represented in type 'int'
    #0 0x7ffb69e87899 in bli_obj_set_comp_dt blis/_src/include/linux-x86_64/blis.h
    #1 0x7ffb69e86f54 in bli_obj_create_without_buffer blis/_src/frame/base/bli_obj.c
    #2 0x7ffb69e87cbe in bli_obj_create_1x1_with_attached_buffer blis/_src/frame/base/bli_obj.c
    #3 0x7ffb69e79ea7 in bli_sgemm_ex blis/_src/frame/3/bli_l3_tapi.c

Looking at the code the offending section seems to be:

static void bli_obj_set_comp_dt( num_t dt, obj_t* obj )
{
    obj->info = ( objbits_t )
                ( obj->info & ~BLIS_COMP_DT_BITS ) |
                ( dt << BLIS_COMP_DT_SHIFT );
}

but I think the actual issues are the defines around things like BLIS_COMP_DT_BITS which is

#define BLIS_COMP_DT_BITS                  ( 0x7  << BLIS_COMP_DT_SHIFT )

e.g. a left shift of 29 to the constant 7 -> which invokes undefined behavior according to clang. What is the intention of this code? To create a big signed wrap around value e.g -536870912?

adrianeboyd commented 3 years ago

Hi, this question is probably better suited for the upstream blis repo. I can find a related issue in their issue tracker (https://github.com/flame/blis/issues/346), but it doesn't look like any changes were made as a result of the discussion. Despite the warning the results should be correct on any modern computer.

It might be possible to address this by changing the definitions like this (this is untested and may not cover all cases):

diff --git a/frame/include/bli_type_defs.h b/frame/include/bli_type_defs.h
index fe030f19..1e7e1a15 100644
--- a/frame/include/bli_type_defs.h
+++ b/frame/include/bli_type_defs.h
@@ -339,41 +339,41 @@ typedef void  (*free_ft)  ( void*  p    );
 //

 // info
-#define BLIS_DATATYPE_BITS                 ( 0x7  << BLIS_DATATYPE_SHIFT )
-#define   BLIS_DOMAIN_BIT                  ( 0x1  << BLIS_DOMAIN_SHIFT )
-#define   BLIS_PRECISION_BIT               ( 0x1  << BLIS_PRECISION_SHIFT )
-#define BLIS_CONJTRANS_BITS                ( 0x3  << BLIS_CONJTRANS_SHIFT )
-#define   BLIS_TRANS_BIT                   ( 0x1  << BLIS_TRANS_SHIFT )
-#define   BLIS_CONJ_BIT                    ( 0x1  << BLIS_CONJ_SHIFT )
-#define BLIS_UPLO_BITS                     ( 0x7  << BLIS_UPLO_SHIFT )
-#define   BLIS_UPPER_BIT                   ( 0x1  << BLIS_UPPER_SHIFT )
-#define   BLIS_DIAG_BIT                    ( 0x1  << BLIS_DIAG_SHIFT )
-#define   BLIS_LOWER_BIT                   ( 0x1  << BLIS_LOWER_SHIFT )
-#define BLIS_UNIT_DIAG_BIT                 ( 0x1  << BLIS_UNIT_DIAG_SHIFT )
-#define BLIS_INVERT_DIAG_BIT               ( 0x1  << BLIS_INVERT_DIAG_SHIFT )
-#define BLIS_TARGET_DT_BITS                ( 0x7  << BLIS_TARGET_DT_SHIFT )
-#define   BLIS_TARGET_DOMAIN_BIT           ( 0x1  << BLIS_TARGET_DOMAIN_SHIFT )
-#define   BLIS_TARGET_PREC_BIT             ( 0x1  << BLIS_TARGET_PREC_SHIFT )
-#define BLIS_EXEC_DT_BITS                  ( 0x7  << BLIS_EXEC_DT_SHIFT )
-#define   BLIS_EXEC_DOMAIN_BIT             ( 0x1  << BLIS_EXEC_DOMAIN_SHIFT )
-#define   BLIS_EXEC_PREC_BIT               ( 0x1  << BLIS_EXEC_PREC_SHIFT )
-#define BLIS_PACK_SCHEMA_BITS              ( 0x7F << BLIS_PACK_SCHEMA_SHIFT )
-#define   BLIS_PACK_RC_BIT                 ( 0x1  << BLIS_PACK_RC_SHIFT )
-#define   BLIS_PACK_PANEL_BIT              ( 0x1  << BLIS_PACK_PANEL_SHIFT )
-#define   BLIS_PACK_FORMAT_BITS            ( 0xF  << BLIS_PACK_FORMAT_SHIFT )
-#define   BLIS_PACK_BIT                    ( 0x1  << BLIS_PACK_SHIFT )
-#define BLIS_PACK_REV_IF_UPPER_BIT         ( 0x1  << BLIS_PACK_REV_IF_UPPER_SHIFT )
-#define BLIS_PACK_REV_IF_LOWER_BIT         ( 0x1  << BLIS_PACK_REV_IF_LOWER_SHIFT )
-#define BLIS_PACK_BUFFER_BITS              ( 0x3  << BLIS_PACK_BUFFER_SHIFT )
-#define BLIS_STRUC_BITS                    ( 0x3  << BLIS_STRUC_SHIFT )
-#define BLIS_COMP_DT_BITS                  ( 0x7  << BLIS_COMP_DT_SHIFT )
-#define   BLIS_COMP_DOMAIN_BIT             ( 0x1  << BLIS_COMP_DOMAIN_SHIFT )
-#define   BLIS_COMP_PREC_BIT               ( 0x1  << BLIS_COMP_PREC_SHIFT )
+#define BLIS_DATATYPE_BITS                 ( 0x7u  << BLIS_DATATYPE_SHIFT )
+#define   BLIS_DOMAIN_BIT                  ( 0x1u  << BLIS_DOMAIN_SHIFT )
+#define   BLIS_PRECISION_BIT               ( 0x1u  << BLIS_PRECISION_SHIFT )
+#define BLIS_CONJTRANS_BITS                ( 0x3u  << BLIS_CONJTRANS_SHIFT )
+#define   BLIS_TRANS_BIT                   ( 0x1u  << BLIS_TRANS_SHIFT )
+#define   BLIS_CONJ_BIT                    ( 0x1u  << BLIS_CONJ_SHIFT )
+#define BLIS_UPLO_BITS                     ( 0x7u  << BLIS_UPLO_SHIFT )
+#define   BLIS_UPPER_BIT                   ( 0x1u  << BLIS_UPPER_SHIFT )
+#define   BLIS_DIAG_BIT                    ( 0x1u  << BLIS_DIAG_SHIFT )
+#define   BLIS_LOWER_BIT                   ( 0x1u  << BLIS_LOWER_SHIFT )
+#define BLIS_UNIT_DIAG_BIT                 ( 0x1u  << BLIS_UNIT_DIAG_SHIFT )
+#define BLIS_INVERT_DIAG_BIT               ( 0x1u  << BLIS_INVERT_DIAG_SHIFT )
+#define BLIS_TARGET_DT_BITS                ( 0x7u  << BLIS_TARGET_DT_SHIFT )
+#define   BLIS_TARGET_DOMAIN_BIT           ( 0x1u  << BLIS_TARGET_DOMAIN_SHIFT )
+#define   BLIS_TARGET_PREC_BIT             ( 0x1u  << BLIS_TARGET_PREC_SHIFT )
+#define BLIS_EXEC_DT_BITS                  ( 0x7u  << BLIS_EXEC_DT_SHIFT )
+#define   BLIS_EXEC_DOMAIN_BIT             ( 0x1u  << BLIS_EXEC_DOMAIN_SHIFT )
+#define   BLIS_EXEC_PREC_BIT               ( 0x1u  << BLIS_EXEC_PREC_SHIFT )
+#define BLIS_PACK_SCHEMA_BITS              ( 0x7Fu << BLIS_PACK_SCHEMA_SHIFT )
+#define   BLIS_PACK_RC_BIT                 ( 0x1u  << BLIS_PACK_RC_SHIFT )
+#define   BLIS_PACK_PANEL_BIT              ( 0x1u  << BLIS_PACK_PANEL_SHIFT )
+#define   BLIS_PACK_FORMAT_BITS            ( 0xFu  << BLIS_PACK_FORMAT_SHIFT )
+#define   BLIS_PACK_BIT                    ( 0x1u  << BLIS_PACK_SHIFT )
+#define BLIS_PACK_REV_IF_UPPER_BIT         ( 0x1u  << BLIS_PACK_REV_IF_UPPER_SHIFT )
+#define BLIS_PACK_REV_IF_LOWER_BIT         ( 0x1u  << BLIS_PACK_REV_IF_LOWER_SHIFT )
+#define BLIS_PACK_BUFFER_BITS              ( 0x3u  << BLIS_PACK_BUFFER_SHIFT )
+#define BLIS_STRUC_BITS                    ( 0x3u  << BLIS_STRUC_SHIFT )
+#define BLIS_COMP_DT_BITS                  ( 0x7u  << BLIS_COMP_DT_SHIFT )
+#define   BLIS_COMP_DOMAIN_BIT             ( 0x1u  << BLIS_COMP_DOMAIN_SHIFT )
+#define   BLIS_COMP_PREC_BIT               ( 0x1u  << BLIS_COMP_PREC_SHIFT )

 // info2
-#define BLIS_SCALAR_DT_BITS                ( 0x7  << BLIS_SCALAR_DT_SHIFT )
-#define   BLIS_SCALAR_DOMAIN_BIT           ( 0x1  << BLIS_SCALAR_DOMAIN_SHIFT )
-#define   BLIS_SCALAR_PREC_BIT             ( 0x1  << BLIS_SCALAR_PREC_SHIFT )
+#define BLIS_SCALAR_DT_BITS                ( 0x7u  << BLIS_SCALAR_DT_SHIFT )
+#define   BLIS_SCALAR_DOMAIN_BIT           ( 0x1u  << BLIS_SCALAR_DOMAIN_SHIFT )
+#define   BLIS_SCALAR_PREC_BIT             ( 0x1u  << BLIS_SCALAR_PREC_SHIFT )

 //
patricksnape commented 3 years ago

That’s a very good point. Sorry I totally blanked that this was just a wrapper around blis. Thanks for pointing me out the related issue. I ended up adding an attribute to the complaining methods to disable the ubsan.