Backport #7982

rb_raise segfaults on %lli format with (0xffffffff + 1)

Added by Erik Chang about 1 year ago. Updated 12 months ago.

[ruby-core:<unknown>]
Status:Closed
Priority:Normal
Assignee:Tomoyuki Chikanaga

Description

This was not a problem on 1.9.3

Run attached "t" extension with:
ruby -rt -e '"".blowup(0xffffffff + 1)'

to reproduce. Using %lld works around the issue.

t.c Magnifier - test extension to segfault (257 Bytes) Erik Chang, 02/28/2013 09:50 AM

extconf.rb Magnifier - extconf for test extension (36 Bytes) Erik Chang, 02/28/2013 09:50 AM

Associated revisions

Revision 40544
Added by Tomoyuki Chikanaga 12 months ago

merge revision(s) 40504,40505,40507,40511: [Backport #7982]

* README.EXT: Add note to warn use of %i in Exceptions [Bug #7982]

* README.EXT: Update note from r40504, by Jeremy Evans [Bug #7982]

* README.EXT: Copy note from r40505 for rb_sprintf() [Bug #7982]

* README.EXT: correct method name to be used.  [Bug #7982]

* README.EXT.ja: add notes too.

History

#1 Updated by Jeremy Evans about 1 year ago

I don't think this is just the %lli format. It is probably at least the %li format as well. home_run's test suite fails on 2.0.0-p0 (working fine on 1.8.7 and 1.9.3), with the following backtrace:

#0 0x0000031c9da03eea in kill () at :2
#1 0x0000031c9da6a6ca in abort () at /usr/src/lib/libc/stdlib/abort.c:70
#2 0x0000031c931a82ad in rbbug () from /usr/local/lib/libruby20.so.0.0
#3 0x0000031c932514bd in sigsegv () from /usr/local/lib/libruby20.so.0.0
#4
#5 0x0000031c9325efd0 in rb
objasstring () from /usr/local/lib/libruby20.so.0.0
#6 0x0000031c93254be8 in ruby_sfvextra () from /usr/local/lib/libruby20.so.0.0
#7 0x0000031c9325310f in BSD
vfprintf () from /usr/local/lib/libruby20.so.0.0
#8 0x0000031c93254eef in rbencvsprintf () from /usr/local/lib/libruby20.so.0.0
#9 0x0000031c931a6f0f in rbraise () from /usr/local/lib/libruby20.so.0.0
#10 0x0000031c9e6d582b in rhrd
scivil (argc=3, argv=0x31c8ffd0358, klass=3421292953680) at ../../../../ext/dateext/dateext.c:1565
1565 rb
raise(rb_eArgError, "invalid date (year: %li, month: %li, day: %li)", year, month, day);

Changing %li to %ld makes it no longer fail at that location (it fails at the next location using %li instead). This is on:

ruby 2.0.0p0 (2013-02-24 revision 39474) [x86_64-openbsd]

#2 Updated by Heesob Park about 1 year ago

Here is a patch:
diff --git a/vsnprintf.c b/vsnprintf.c.new
index 1e4cdc5..29bdfda 100644
--- a/vsnprintf.c
+++ b/vsnprintf.c.new
@@ -809,10 +809,10 @@ reswitch: switch (ch) {
if (fp->vextra && (flags & INTPTRMASK) == INTPTRFLAG) {
FLUSH();
#if defined HAVESANEQUAD && SIZEOFVOIDP == SIZEOFLONGLONG
- uqval = va
arg(ap, uquadt);
+ uqval = LL2NUM(vaarg(ap, uquadt));
cp = (*fp->vextra)(fp, sizeof(uqval), &uqval, &fieldsz, sign);
#else
- ulval = va
arg(ap, ulong);
+ ulval = LONG2NUM(va
arg(ap, u_long));
cp = (*fp->vextra)(fp, sizeof(ulval), &ulval, &fieldsz, sign);
#endif
sign = '\0';

#3 Updated by Nobuyoshi Nakada about 1 year ago

  • Status changed from Open to Rejected

=begin
Sorry, you can't use (({%i})) for (({rbraise()})), (({rbwarn()})), and so on.
It's now used for (({Object}))s.

Use (({%d})) instead.
=end

#4 Updated by Jeremy Evans about 1 year ago

On 02/28 02:59, nobu (Nobuyoshi Nakada) wrote:

Issue #7982 has been updated by nobu (Nobuyoshi Nakada).

Status changed from Open to Rejected

=begin
Sorry, you can't use (({%i})) for (({rbraise()})), (({rbwarn()})), and so on.
It's now used for (({Object}))s.

Use (({%d})) instead.
=end

The Changelog entry for this is:

  • vsnprintf.c (BSDvfprintf): [EXPERIMENTAL] object representation in
    rb
    enc_vsprintf(). [Feature #5896]

    There is no indication that %i is now handled differently than before.
    For a C programmer, overriding %i which has historically been used for
    integers is very confusing and error prone. It is true that ruby is
    not C, but this is a C API, and rb_raise's documentation in README.EXT
    states:

    void rbraise(rbeRuntimeError, const char *fmt, ...) ::

    Raises RuntimeError. The fmt is a format string just like printf().

    I content that the format string syntax in ruby 2.0.0 is not "just
    like printf()". Maybe "similar to printf(), with the following
    exceptions: ..."?

    As this is still marked EXPERIMENTAL, it would be better if another
    letter that isn't already supported by printf(3) was used instead.
    At the very least, if you plan to continue the use of %i for object
    representation, there should be specific notes in the documentation.

    Thanks,
    Jeremy

#5 Updated by Nobuyoshi Nakada about 1 year ago

  • Category set to doc
  • Status changed from Rejected to Open

=begin
: jeremyevans (Jeremy Evans) wrote:
I content that the format string syntax in ruby 2.0.0 is not "just
like printf()". Maybe "similar to printf(), with the following
exceptions: ..."?

Thank you, I'll add it later.
=end

#6 Updated by Zachary Scott 12 months ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r40504.
Erik, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • README.EXT: Add note to warn use of %i in Exceptions [Bug #7982]

#7 Updated by Zachary Scott 12 months ago

  • Status changed from Closed to Assigned
  • Assignee set to Nobuyoshi Nakada
  • % Done changed from 100 to 50

@nobu, how does this look? Any other places we should add this? Are there any more formats that should be avoided?

#8 Updated by Zachary Scott 12 months ago

  • Backport set to 1.9.3: UNKNOWN, 2.0.0: UNKNOWN

#9 Updated by Zachary Scott 12 months ago

  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 2.0.0: UNKNOWN

#10 Updated by Zachary Scott 12 months ago

  • Status changed from Assigned to Closed
  • % Done changed from 50 to 100

This issue was solved with changeset r40507.
Erik, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • README.EXT: Copy note from r40505 for rb_sprintf() [Bug #7982]

#11 Updated by Zachary Scott 12 months ago

  • Status changed from Closed to Assigned
  • Assignee changed from Nobuyoshi Nakada to Tomoyuki Chikanaga
  • % Done changed from 100 to 50

How is this? We should backport this too

#12 Updated by Nobuyoshi Nakada 12 months ago

  • % Done changed from 50 to 100
  • Status changed from Assigned to Closed

This issue was solved with changeset r40511.
Erik, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


README.EXT: update note

  • README.EXT: correct method name to be used. [Bug #7982]
  • README.EXT.ja: add notes too.

#13 Updated by Tomoyuki Chikanaga 12 months ago

  • Tracker changed from Bug to Backport
  • Project changed from ruby-trunk to Backport200
  • Category deleted (doc)
  • Status changed from Closed to Assigned
  • Target version deleted (2.1.0)

#14 Updated by Tomoyuki Chikanaga 12 months ago

  • Status changed from Assigned to Closed

This issue was solved with changeset r40544.
Erik, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


merge revision(s) 40504,40505,40507,40511: [Backport #7982]

* README.EXT: Add note to warn use of %i in Exceptions [Bug #7982]

* README.EXT: Update note from r40504, by Jeremy Evans [Bug #7982]

* README.EXT: Copy note from r40505 for rb_sprintf() [Bug #7982]

* README.EXT: correct method name to be used.  [Bug #7982]

* README.EXT.ja: add notes too.

Also available in: Atom PDF