Project

General

Profile

Bug #12191

Violation of ANSI aliasing rules causing problems while compiling

Added by Zarko (Zarko Todorovski) about 1 year ago. Updated 9 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:74427]

Description

Hi, I work with IBM's XL compiler and we're noticing that there we're getting compile time failures due to ANSI aliasing rule violations.

For example, in https://github.com/ruby/ruby/blob/trunk/sprintf.c This function:

rb_str_vcatf(VALUE str, const char *fmt, va_list ap)
{
    rb_printf_buffer_extra buffer;
#define f buffer.base
    VALUE klass;

    StringValue(str);
    rb_str_modify(str);
    f._flags = __SWR | __SSTR;
    f._bf._size = 0;
    f._w = rb_str_capacity(str);
    f._bf._base = (unsigned char *)str;
**    f._p = (unsigned char *)RSTRING_END(str);
    klass = RBASIC(str)->klass;
    RBASIC_CLEAR_CLASS(str);
**    f.vwrite = ruby__sfvwrite;
    f.vextra = ruby__sfvextra;
    buffer.value = 0;
    BSD_vfprintf(&f, fmt, ap);
    RBASIC_SET_CLASS_RAW(str, klass);
    rb_str_resize(str, (char *)f._p - RSTRING_PTR(str));
#undef f

    return str;
}

When the bolded macros are expanded, they look like this:

include/ruby/ruby.h:869:#define RSTRING_END(str) \
include/ruby/ruby.h-870-    (!(RBASIC(str)->flags & RSTRING_NOEMBED) ? \
include/ruby/ruby.h-871-     (RSTRING(str)->as.ary + RSTRING_EMBED_LEN(str)) : \
include/ruby/ruby.h-872-     (RSTRING(str)->as.heap.ptr + RSTRING(str)->as.heap.len))

include/ruby/ruby.h:1086:#define RSTRING(obj) (R_CAST(RString)(obj))

include/ruby/ruby.h:1082:#define RBASIC(obj)  (R_CAST(RBasic)(obj))

include/ruby/ruby.h:1081:#define R_CAST(st)   (struct st*)

internal.h:852:#define RBASIC_CLEAR_CLASS(obj)        (((struct RBasicRaw *)((VALUE)(obj)))->klass = 0)

The function violates the ANSI aliasing rule since it takes an unsigned long, casts it to a pointer to either RBasic or RBasicRaw and then dereferences it. (RBasic).klass and (RBasicRaw).klass both alias unsigned long, but not each other, as RBasic and RBasicRaw are different types.

Additionally, other functions in sprintf.c also seem to have aliasing violations.

A fix such as changing line https://github.com/ruby/ruby/blob/trunk/internal.h#L1063 from

#define RBASIC_CLEAR_CLASS(obj)        (((struct RBasicRaw *)((VALUE)(obj)))->klass = 0)

To:

#ifdef __ibmxl__
#define RBASIC_CLEAR_CLASS(obj)        memset(&(((struct RBasicRaw *)((VALUE)(obj)))->klass), 0, sizeof(((struct RBasicRaw *)((VALUE)(obj)))->klass))
#else
#define RBASIC_CLEAR_CLASS(obj)        (((struct RBasicRaw *)((VALUE)(obj)))->klass = 0)
#endif

but there should be no need to make a special case for the XL compiler as it's following the ANSI aliasing rules.


Related issues

Related to Ruby trunk - Bug #10802: strict-aliasing warning on RHEL/CentOS 6 Closed
Related to Ruby trunk - Bug #12657: [PATCH] ANSI aliasing fix for XL compiler Closed

Associated revisions

Revision 55831
Added by shyouhei (Shyouhei Urabe) 9 months ago

  • internal.h (RBASIC_CLEAR_CLASS): Reroute ANSI C's strict aliasing rule. [Bug #12191][Bug #12657]

Revision 55831
Added by shyouhei (Shyouhei Urabe) 9 months ago

  • internal.h (RBASIC_CLEAR_CLASS): Reroute ANSI C's strict aliasing rule. [Bug #12191][Bug #12657]

History

#1 [ruby-core:74428] Updated by shyouhei (Shyouhei Urabe) about 1 year ago

  • Description updated (diff)

#2 [ruby-core:74799] Updated by shyouhei (Shyouhei Urabe) about 1 year ago

Does this help? RBasic and RBasicRaw does in fact occupy identical memory region so making them union seems the most natural way.

diff --git a/internal.h b/internal.h
index 3970431..498a7e3 100644
--- a/internal.h
+++ b/internal.h
@@ -1099,9 +1099,12 @@ NORETURN(void rb_undefined_alloc(VALUE klass));
 double rb_num_to_dbl(VALUE val);
 VALUE rb_obj_dig(int argc, VALUE *argv, VALUE self, VALUE notfound);

-struct RBasicRaw {
-    VALUE flags;
-    VALUE klass;
+union RBasicCast {
+    struct RBasic opaque;
+    struct RBasicRaw {
+        VALUE flags;
+        VALUE klass;
+    } transparent;
 };

 #define RBASIC_CLEAR_CLASS(obj)        (((struct RBasicRaw *)((VALUE)(obj)))->klass = 0)

#3 [ruby-core:74841] Updated by Zarko (Zarko Todorovski) about 1 year ago

Thanks for making the changed but we've tried getting it to work and it doesn't seem to. It doesn't look like setting these two structs in a union has any impact on aliasing between ("(RBasicRaw).klass") and {"(RBasic).klass"} in this case.

#4 Updated by shyouhei (Shyouhei Urabe) 10 months ago

  • Related to Bug #10802: strict-aliasing warning on RHEL/CentOS 6 added

#5 Updated by shyouhei (Shyouhei Urabe) 9 months ago

  • Status changed from Open to Closed

Applied in changeset r55831.


  • internal.h (RBASIC_CLEAR_CLASS): Reroute ANSI C's strict aliasing rule. [Bug #12191][Bug #12657]

#6 Updated by vo.x (Vit Ondruch) 6 months ago

  • Related to Bug #12657: [PATCH] ANSI aliasing fix for XL compiler added

Also available in: Atom PDF