Skip to content

Commit

Permalink
Merge pull request #4380 from jruby/truffle-sprintf-debug-arg-check
Browse files Browse the repository at this point in the history 
[Truffle] Sprintf check for too many args when $DEBUG is true
  • Loading branch information
@bjfish
bjfish committed Dec 13, 2016
2 parents e3fc025 + b21ffa5 commit 37e90a2
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 12 deletions.
1 change: 0 additions & 1 deletion spec/truffle/tags/core/string/modulo_tags.txt
View file
@@ -1,4 +1,3 @@
fails:String#% raises an ArgumentError for unused arguments when $DEBUG is true
fails:String#% behaves as if calling Kernel#Float for %e arguments, when the passed argument does not respond to #to_ary
fails:String#% behaves as if calling Kernel#Float for %e arguments, when the passed argument is hexadecimal string
fails:String#% behaves as if calling Kernel#Float for %E arguments, when the passed argument does not respond to #to_ary
Expand Down
4 changes: 2 additions & 2 deletions truffle/src/main/java/org/jruby/truffle/core/format/printf/PrintfCompiler.java
View file
Expand Up @@ -28,8 +28,8 @@ public PrintfCompiler(RubyContext context, RubyNode currentNode) {
this.currentNode = currentNode;
}

public CallTarget compile(byte[] format, Object[] arguments) {
final PrintfSimpleParser parser = new PrintfSimpleParser(bytesToChars(format), arguments);
public CallTarget compile(byte[] format, Object[] arguments, boolean isDebug) {
final PrintfSimpleParser parser = new PrintfSimpleParser(bytesToChars(format), arguments, isDebug);
final List<SprintfConfig> configs = parser.parse();
final PrintfSimpleTreeBuilder builder = new PrintfSimpleTreeBuilder(context, configs);

Expand Down
18 changes: 17 additions & 1 deletion truffle/src/main/java/org/jruby/truffle/core/format/printf/PrintfSimpleParser.java
View file
Expand Up @@ -19,10 +19,12 @@ public class PrintfSimpleParser {

private final char[] source;
private final Object[] arguments;
private final boolean isDebug;

public PrintfSimpleParser(char[] source, Object[] arguments) {
public PrintfSimpleParser(char[] source, Object[] arguments, boolean isDebug) {
this.source = source;
this.arguments = arguments;
this.isDebug = isDebug;
}

@SuppressWarnings("fallthrough")
Expand All @@ -31,6 +33,7 @@ public List<SprintfConfig> parse() {
ArgType argType = ArgType.NONE;

final int end = source.length;
int argCount = 0;

for (int i = 0; i < end; ) {

Expand Down Expand Up @@ -152,6 +155,7 @@ public List<SprintfConfig> parse() {
i = numberDollarWidth.getNextI();
} else {
checkNextArg(argType, 1); // TODO index next args
argCount += 1;
argType = ArgType.UNNUMBERED;
config.setWidthStar(true);
i++;
Expand All @@ -172,6 +176,7 @@ public List<SprintfConfig> parse() {
i = numberDollar.getNextI();
} else {
checkNextArg(argType, 1); // TODO idx
argCount += 1;
argType = ArgType.UNNUMBERED;
config.setPrecisionStar(true);
i += 2;
Expand Down Expand Up @@ -202,6 +207,7 @@ public List<SprintfConfig> parse() {
i++;
if (!argTypeSet) {
checkNextArg(argType, 1);
argCount += 1;
argType = ArgType.UNNUMBERED;
}
finished = true;
Expand All @@ -213,6 +219,7 @@ public List<SprintfConfig> parse() {
i++;
if (!argTypeSet) { // Speculative
checkNextArg(argType, 1);
argCount += 1;
argType = ArgType.UNNUMBERED;
}
finished = true;
Expand All @@ -227,6 +234,7 @@ public List<SprintfConfig> parse() {
case 'u':
if (!argTypeSet) {
checkNextArg(argType, 1); // TODO idx correctly
argCount += 1;
argType = ArgType.UNNUMBERED;
}
config.setFormatType(SprintfConfig.FormatType.INTEGER);
Expand All @@ -243,6 +251,7 @@ public List<SprintfConfig> parse() {
case 'f':
if (!argTypeSet) {
checkNextArg(argType, 1);
argCount += 1;
argType = ArgType.UNNUMBERED;
}
config.setFormatType(SprintfConfig.FormatType.FLOAT);
Expand All @@ -255,6 +264,13 @@ public List<SprintfConfig> parse() {
}
}
}
if ((argType == ArgType.UNNUMBERED || argType == ArgType.NONE) &&
arguments.length > argCount) {
if (isDebug) {
throw new InvalidFormatException("too many arguments for format string");
}
}

return configs;
}

Expand Down
27 changes: 22 additions & 5 deletions truffle/src/main/java/org/jruby/truffle/core/kernel/KernelNodes.java
View file
Expand Up @@ -58,6 +58,8 @@
import org.jruby.truffle.core.basicobject.BasicObjectNodes.ReferenceEqualNode;
import org.jruby.truffle.core.basicobject.BasicObjectNodesFactory;
import org.jruby.truffle.core.binding.BindingNodes;
import org.jruby.truffle.core.cast.BooleanCastNode;
import org.jruby.truffle.core.cast.BooleanCastNodeGen;
import org.jruby.truffle.core.cast.BooleanCastWithDefaultNodeGen;
import org.jruby.truffle.core.cast.DurationToMillisecondsNodeGen;
import org.jruby.truffle.core.cast.NameToJavaStringNode;
Expand Down Expand Up @@ -109,6 +111,8 @@
import org.jruby.truffle.language.dispatch.DoesRespondDispatchHeadNode;
import org.jruby.truffle.language.dispatch.MissingBehavior;
import org.jruby.truffle.language.dispatch.RubyCallNode;
import org.jruby.truffle.language.globals.ReadGlobalVariableNode;
import org.jruby.truffle.language.globals.ReadGlobalVariableNodeGen;
import org.jruby.truffle.language.loader.CodeLoader;
import org.jruby.truffle.language.loader.RequireNode;
import org.jruby.truffle.language.loader.SourceLoader;
Expand Down Expand Up @@ -1837,18 +1841,25 @@ public abstract static class SprintfNode extends CoreMethodArrayArgumentsNode {

@Child private RopeNodes.MakeLeafRopeNode makeLeafRopeNode;
@Child private TaintNode taintNode;
@Child private BooleanCastNode readDebugGlobalNode;

private final BranchProfile exceptionProfile = BranchProfile.create();
private final ConditionProfile resizeProfile = ConditionProfile.createBinaryProfile();

@Specialization(guards = { "isRubyString(format)", "ropesEqual(format, cachedFormat)" })
public SprintfNode(RubyContext context, SourceSection sourceSection) {
super(context, sourceSection);
this.readDebugGlobalNode = BooleanCastNodeGen.create(ReadGlobalVariableNodeGen.create("$DEBUG"));
}

@Specialization(guards = { "isRubyString(format)", "ropesEqual(format, cachedFormat)", "isDebug(frame) == cachedIsDebug" })
public DynamicObject formatCached(
VirtualFrame frame,
DynamicObject format,
Object[] arguments,
@Cached("isDebug(frame)") boolean cachedIsDebug,
@Cached("privatizeRope(format)") Rope cachedFormat,
@Cached("ropeLength(cachedFormat)") int cachedFormatLength,
@Cached("create(compileFormat(format, arguments))") DirectCallNode callPackNode) {
@Cached("create(compileFormat(format, arguments, isDebug(frame)))") DirectCallNode callPackNode) {
final BytesResult result;

try {
Expand All @@ -1870,8 +1881,10 @@ public DynamicObject formatUncached(
@Cached("create()") IndirectCallNode callPackNode) {
final BytesResult result;

final boolean isDebug = readDebugGlobalNode.executeBoolean(frame);

try {
result = (BytesResult) callPackNode.call(frame, compileFormat(format, arguments),
result = (BytesResult) callPackNode.call(frame, compileFormat(format, arguments, isDebug),
new Object[]{ arguments, arguments.length });
} catch (FormatException e) {
exceptionProfile.enter();
Expand Down Expand Up @@ -1912,17 +1925,21 @@ private DynamicObject finishFormat(int formatLength, BytesResult result) {
}

@TruffleBoundary
protected CallTarget compileFormat(DynamicObject format, Object[] arguments) {
protected CallTarget compileFormat(DynamicObject format, Object[] arguments, boolean isDebug) {
assert RubyGuards.isRubyString(format);

try {
return new PrintfCompiler(getContext(), this)
.compile(Layouts.STRING.getRope(format).getBytes(), arguments);
.compile(Layouts.STRING.getRope(format).getBytes(), arguments, isDebug);
} catch (InvalidFormatException e) {
throw new RaiseException(coreExceptions().argumentError(e.getMessage(), this));
}
}

protected boolean isDebug(VirtualFrame frame) {
return readDebugGlobalNode.executeBoolean(frame);
}

}

@Primitive(name = "kernel_global_variables")
Expand Down
3 changes: 1 addition & 2 deletions truffle/src/main/java/org/jruby/truffle/language/globals/ReadGlobalVariableNode.java
View file
Expand Up @@ -20,8 +20,7 @@ public abstract class ReadGlobalVariableNode extends RubyNode {

private final String name;

public ReadGlobalVariableNode(RubyContext context, SourceSection sourceSection, String name) {
super(context, sourceSection);
public ReadGlobalVariableNode(String name) {
this.name = name;
}

Expand Down
2 changes: 1 addition & 1 deletion truffle/src/main/java/org/jruby/truffle/parser/BodyTranslator.java
View file
Expand Up @@ -1832,7 +1832,7 @@ public RubyNode visitGlobalVarNode(GlobalVarParseNode node) {
// Instead, it reads the backtrace field of the thread-local $! value.
ret = new ReadLastBacktraceNode(context, fullSourceSection);
} else {
ret = ReadGlobalVariableNodeGen.create(context, fullSourceSection, name);
ret = ReadGlobalVariableNodeGen.create(name);
}

return addNewlineIfNeeded(node, ret);
Expand Down

0 comments on commit 37e90a2

Please sign in to comment.