Backport #7982
closed
rb_raise segfaults on %lli format with (0xffffffff + 1)
Added by erik.s.chang (Erik Chang) over 11 years ago. Updated over 11 years ago.
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.
Files
| t.c (257 Bytes) t.c | test extension to segfault | erik.s.chang (Erik Chang), 02/28/2013 09:50 AM | |
| extconf.rb (36 Bytes) extconf.rb | extconf for test extension | erik.s.chang (Erik Chang), 02/28/2013 09:50 AM |
Updated by jeremyevans0 (Jeremy Evans) over 11 years 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 rb_bug () 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_obj_as_string () 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 rb_enc_vsprintf () from /usr/local/lib/libruby20.so.0.0
#9 0x0000031c931a6f0f in rb_raise () from /usr/local/lib/libruby20.so.0.0
#10 0x0000031c9e6d582b in rhrd_s_civil (argc=3, argv=0x31c8ffd0358, klass=3421292953680) at ../../../../ext/date_ext/date_ext.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]
Updated by phasis68 (Heesob Park) over 11 years 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 & INTPTR_MASK) == INTPTR_FLAG) {
FLUSH();
#if defined HAVE_SANE_QUAD && SIZEOF_VOIDP == SIZEOF_LONG_LONG
-
uqval = va_arg(ap, u_quad_t);
-
uqval = LL2NUM(va_arg(ap, u_quad_t)); cp = (*fp->vextra)(fp, sizeof(uqval), &uqval, &fieldsz, sign);
#else
-
ulval = va_arg(ap, u_long);
-
ulval = LONG2NUM(va_arg(ap, u_long)); cp = (*fp->vextra)(fp, sizeof(ulval), &ulval, &fieldsz, sign);
#endif
sign = '\0';
Updated by nobu (Nobuyoshi Nakada) over 11 years ago
- Status changed from Open to Rejected
=begin
Sorry, you can't use (({%i})) for (({rb_raise()})), (({rb_warn()})), and so on.
It's now used for (({Object}))s.
Use (({%d})) instead.
=end
Updated by jeremyevans (Jeremy Evans) over 11 years 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 (({rb_raise()})), (({rb_warn()})), and so on.
It's now used for (({Object}))s.Use (({%d})) instead.
=end
The Changelog entry for this is:
- vsnprintf.c (BSD_vfprintf): [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 rb_raise(rb_eRuntimeError, 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
Updated by nobu (Nobuyoshi Nakada) over 11 years 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
Updated by zzak (zzak _) over 11 years 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]
Updated by zzak (zzak _) over 11 years ago
- Status changed from Closed to Assigned
- Assignee set to nobu (Nobuyoshi Nakada)
- % Done changed from 100 to 50
@nobu (Nobuyoshi Nakada), how does this look? Any other places we should add this? Are there any more formats that should be avoided?
Updated by zzak (zzak _) over 11 years ago
- Backport set to 1.9.3: UNKNOWN, 2.0.0: UNKNOWN
Updated by zzak (zzak _) over 11 years ago
- Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 2.0.0: UNKNOWN
Updated by zzak (zzak _) over 11 years 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]
Updated by zzak (zzak _) over 11 years ago
- Status changed from Closed to Assigned
- Assignee changed from nobu (Nobuyoshi Nakada) to nagachika (Tomoyuki Chikanaga)
- % Done changed from 100 to 50
How is this? We should backport this too
Updated by nobu (Nobuyoshi Nakada) over 11 years 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.
Updated by nagachika (Tomoyuki Chikanaga) over 11 years ago
- Tracker changed from Bug to Backport
- Project changed from Ruby master to Backport200
- Category deleted (
doc) - Status changed from Closed to Assigned
- Target version deleted (
2.1.0)
Updated by nagachika (Tomoyuki Chikanaga) over 11 years 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.