From 0748cd24c6261c67fc55180103259b763ba6108b Mon Sep 17 00:00:00 2001 From: Marc-Andre Laperle Date: Mon, 7 Jun 2021 01:17:43 -0400 Subject: [PATCH] Bug 573764 - Ambiguous conversion on numeric type Starting with C++11, a null pointer constant has to be specifically an integer literal of 0 and not any constant expression. Before this change, an expression like (0 & 1) would wrongly being considered a null pointer constant. It also means it could implicitly convert to a pointer type (like int *) and lead to problems during function resolution, like ambiguity. This change corrects the behavior for C++11 by tracking whether the integer type (basic type) came from a literal expression so that we can add this additional constraint when checking for a null pointer constant. Because types are sometimes returned directly when evaluating different kinds of non-literal expressions that contain literal expressions, we have to be careful that we remove the flag that tracks "from literal expression". Unfortunately, the semantic code does not track the active C++ version which means the behavior for pre-C++11 will be impacted. Tracking the active C++ version would not be trivial and at least the new behavior is more future-proof. Change-Id: Ied625e96e70390872e36ab5bb4dc238d75809d2e Signed-off-by: Marc-Andre Laperle --- .../core/parser/tests/ast2/AST2CPPTests.java | 65 +++++++++++++++++ .../org.eclipse.cdt.core/META-INF/MANIFEST.MF | 2 +- .../core/dom/parser/cpp/CPPBasicType.java | 72 +++++++++++++++---- .../dom/parser/cpp/semantics/Conversions.java | 10 ++- .../dom/parser/cpp/semantics/EvalBinary.java | 5 ++ .../dom/parser/cpp/semantics/EvalComma.java | 4 ++ .../parser/cpp/semantics/EvalConditional.java | 9 +++ .../eclipse/cdt/internal/core/pdom/PDOM.java | 9 ++- 8 files changed, 157 insertions(+), 19 deletions(-) diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2CPPTests.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2CPPTests.java index bcf3f344e29..cc49aad7c9b 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2CPPTests.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2CPPTests.java @@ -13658,4 +13658,69 @@ public class AST2CPPTests extends AST2CPPTestBase { parseAndCheckBindings(getAboveComment(), CPP, true); } + // void function(int * a) { } + // void function(unsigned int b) { } + // + // void functionPtr(decltype(nullptr) c) { } + // + // int main() + // { + // function(0); // 0 + // function(nullptr); // 1 + // functionPtr(0); // 2 + // functionPtr(nullptr); // 3 + // + // function(0 & 1); // 4 + // functionPtr(0 & 1); // 5 + // + // function(0 << 1); // 6 + // functionPtr(0 << 1); // 7 + // + // function((0,0)); // 8 + // functionPtr((0,0)); // 9 + // + // function(int{0}); // 10 + // functionPtr(int{0}); // 11 + // + // function(true ? 0 : 0); // 12 + // functionPtr(true ? 0 : 0); // 13 + // } + public void testNullPointerConstantConversion_573764() throws Exception { + IASTTranslationUnit tu = parse(getAboveComment(), CPP); + NameCollector collector = new NameCollector(); + tu.accept(collector); + + IBinding funcDeclA = collector.getName(0).resolveBinding(); + IBinding funcDeclB = collector.getName(2).resolveBinding(); + IBinding funcDeclC = collector.getName(4).resolveBinding(); + + int callIndexStart = 7; + // Ambiguous + assertTrue(collector.getName(callIndexStart + 0).resolveBinding() instanceof IProblemBinding); + assertEquals(funcDeclA, collector.getName(callIndexStart + 1).resolveBinding()); + assertEquals(funcDeclC, collector.getName(callIndexStart + 2).resolveBinding()); + assertEquals(funcDeclC, collector.getName(callIndexStart + 3).resolveBinding()); + + assertEquals(funcDeclB, collector.getName(callIndexStart + 4).resolveBinding()); + // Invalid argument (not null pointer constant) + assertTrue(collector.getName(callIndexStart + 5).resolveBinding() instanceof IProblemBinding); + + assertEquals(funcDeclB, collector.getName(callIndexStart + 6).resolveBinding()); + // Invalid argument (not null pointer constant) + assertTrue(collector.getName(callIndexStart + 7).resolveBinding() instanceof IProblemBinding); + + // EvalComma + assertEquals(funcDeclB, collector.getName(callIndexStart + 8).resolveBinding()); + // Invalid argument (not null pointer constant) + assertTrue(collector.getName(callIndexStart + 9).resolveBinding() instanceof IProblemBinding); + + assertEquals(funcDeclB, collector.getName(callIndexStart + 10).resolveBinding()); + // Invalid argument (not null pointer constant) + assertTrue(collector.getName(callIndexStart + 11).resolveBinding() instanceof IProblemBinding); + + // EvalConditional + assertEquals(funcDeclB, collector.getName(callIndexStart + 12).resolveBinding()); + // Invalid argument (not null pointer constant) + assertTrue(collector.getName(callIndexStart + 13).resolveBinding() instanceof IProblemBinding); + } } diff --git a/core/org.eclipse.cdt.core/META-INF/MANIFEST.MF b/core/org.eclipse.cdt.core/META-INF/MANIFEST.MF index b40557575b1..d692c5c25be 100644 --- a/core/org.eclipse.cdt.core/META-INF/MANIFEST.MF +++ b/core/org.eclipse.cdt.core/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: %pluginName Bundle-SymbolicName: org.eclipse.cdt.core; singleton:=true -Bundle-Version: 7.2.100.qualifier +Bundle-Version: 7.2.200.qualifier Bundle-Activator: org.eclipse.cdt.core.CCorePlugin Bundle-Vendor: %providerName Bundle-Localization: plugin diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPBasicType.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPBasicType.java index c26dde91be6..d51a95775a9 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPBasicType.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPBasicType.java @@ -17,6 +17,9 @@ package org.eclipse.cdt.internal.core.dom.parser.cpp; import static org.eclipse.cdt.core.dom.ast.cpp.ICPPParameter.EMPTY_CPPPARAMETER_ARRAY; +import java.text.MessageFormat; + +import org.eclipse.cdt.core.CCorePlugin; import org.eclipse.cdt.core.dom.ast.ASTTypeUtil; import org.eclipse.cdt.core.dom.ast.IASTExpression; import org.eclipse.cdt.core.dom.ast.IASTLiteralExpression; @@ -58,10 +61,16 @@ public class CPPBasicType implements ICPPBasicType, ISerializableType { public static final CPPBasicType CHAR = new CPPBasicType(Kind.eChar, 0); public static final CPPBasicType VOID = new CPPBasicType(Kind.eVoid, 0); - private static final int FROM_STRING_LITERAL = 1 << 31; + public static final int FROM_LITERAL = 1 << 30; + public static final int FROM_STRING_LITERAL = 1 << 31; + + private static final short TYPE_BUFFER_KIND_OFFSET = ITypeMarshalBuffer.FIRST_FLAG; + private static final short TYPE_BUFFER_FROM_LITERAL_FLAG = ITypeMarshalBuffer.SECOND_LAST_FLAG / 2; + private static final short TYPE_BUFFER_FIRST_FLAG_AFTER_KIND = TYPE_BUFFER_FROM_LITERAL_FLAG; + private static final int MAX_KIND_INT_VALUE = (TYPE_BUFFER_FIRST_FLAG_AFTER_KIND - 1) / TYPE_BUFFER_KIND_OFFSET; private final Kind fKind; - private final int fModifiers; + private int fModifiers; private Long fAssociatedValue; private ICPPFunction fPseudoDestructor; @@ -77,9 +86,11 @@ public class CPPBasicType implements ICPPBasicType, ISerializableType { } else { fKind = kind; } - if (expression instanceof IASTLiteralExpression - && ((IASTLiteralExpression) expression).getKind() == IASTLiteralExpression.lk_string_literal) { - qualifiers |= FROM_STRING_LITERAL; + if (expression instanceof IASTLiteralExpression) { + qualifiers |= FROM_LITERAL; + if (((IASTLiteralExpression) expression).getKind() == IASTLiteralExpression.lk_string_literal) { + qualifiers |= FROM_STRING_LITERAL; + } } fModifiers = qualifiers; if (expression instanceof ICPPASTInitializerClause) { @@ -209,9 +220,19 @@ public class CPPBasicType implements ICPPBasicType, ISerializableType { @Override public CPPBasicType clone() { + return clone(~0); + } + + /** + * Clone as normal but keep only requested flags. + * + * @param flagsMask The mask of flags to preserve during the clone. + */ + public CPPBasicType clone(int flagsMask) { CPPBasicType t = null; try { t = (CPPBasicType) super.clone(); + t.fModifiers &= flagsMask; } catch (CloneNotSupportedException e) { // Not going to happen. } @@ -235,15 +256,22 @@ public class CPPBasicType implements ICPPBasicType, ISerializableType { } /** - * Returns {@code true} if the type was created for a string literal. + * Returns {@code true} if the type was created from a string literal. */ public final boolean isFromStringLiteral() { return (fModifiers & FROM_STRING_LITERAL) != 0; } + /** + * Returns {@code true} if the type was created from a literal. + */ + public final boolean isFromLiteral() { + return (fModifiers & FROM_LITERAL) != 0; + } + @Override public final int getModifiers() { - return fModifiers & ~FROM_STRING_LITERAL; + return fModifiers & ~FROM_STRING_LITERAL & ~FROM_LITERAL; } @Override @@ -254,27 +282,47 @@ public class CPPBasicType implements ICPPBasicType, ISerializableType { @Override public void marshal(ITypeMarshalBuffer buffer) throws CoreException { final int kind = getKind().ordinal(); - final int shiftedKind = kind * ITypeMarshalBuffer.FIRST_FLAG; + // 'kind' uses the space of the first few flags so make sure it doesn't overflow to the actual used flags further. + if (kind > MAX_KIND_INT_VALUE) { + throw new CoreException(CCorePlugin.createStatus( + MessageFormat.format("Cannot marshal a basic type, kind ''{0}'' would overflow following flags.", //$NON-NLS-1$ + getKind().toString()))); + } + final int shiftedKind = kind * TYPE_BUFFER_KIND_OFFSET; final int modifiers = getModifiers(); short firstBytes = (short) (ITypeMarshalBuffer.BASIC_TYPE | shiftedKind); - if (modifiers != 0) - firstBytes |= ITypeMarshalBuffer.LAST_FLAG; + if (isFromLiteral()) + firstBytes = setFirstBytesFlag(firstBytes, TYPE_BUFFER_FROM_LITERAL_FLAG); if (fAssociatedValue != null) - firstBytes |= ITypeMarshalBuffer.SECOND_LAST_FLAG; + firstBytes = setFirstBytesFlag(firstBytes, ITypeMarshalBuffer.SECOND_LAST_FLAG); + if (modifiers != 0) + firstBytes = setFirstBytesFlag(firstBytes, ITypeMarshalBuffer.LAST_FLAG); buffer.putShort(firstBytes); if (modifiers != 0) buffer.putByte((byte) modifiers); if (fAssociatedValue != null) buffer.putLong(getAssociatedNumericalValue()); + + } + + private static short setFirstBytesFlag(short firstBytes, short flag) throws CoreException { + if (flag < TYPE_BUFFER_FIRST_FLAG_AFTER_KIND) { + throw new CoreException(CCorePlugin.createStatus( + MessageFormat.format("Cannot marshal a basic type, flag ''0x{0}'' overlaps ''kind'' bytes.", //$NON-NLS-1$ + Integer.toHexString(flag)))); + } + return (short) (firstBytes | flag); } public static IType unmarshal(short firstBytes, ITypeMarshalBuffer buffer) throws CoreException { final boolean haveModifiers = (firstBytes & ITypeMarshalBuffer.LAST_FLAG) != 0; final boolean haveAssociatedNumericalValue = (firstBytes & ITypeMarshalBuffer.SECOND_LAST_FLAG) != 0; int modifiers = 0; - int kind = (firstBytes & (ITypeMarshalBuffer.SECOND_LAST_FLAG - 1)) / ITypeMarshalBuffer.FIRST_FLAG; + int kind = (firstBytes & (TYPE_BUFFER_FIRST_FLAG_AFTER_KIND - 1)) / TYPE_BUFFER_KIND_OFFSET; if (haveModifiers) modifiers = buffer.getByte(); + if ((firstBytes & TYPE_BUFFER_FROM_LITERAL_FLAG) != 0) + modifiers |= FROM_LITERAL; CPPBasicType result = new CPPBasicType(Kind.values()[kind], modifiers); if (haveAssociatedNumericalValue) result.setAssociatedNumericalValue(buffer.getLong()); diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/Conversions.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/Conversions.java index b4858bbd4b4..57c315ef531 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/Conversions.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/Conversions.java @@ -1168,9 +1168,13 @@ public class Conversions { if (basicType.getKind() == Kind.eNullPtr) return true; - Long val = basicType.getAssociatedNumericalValue(); - if (val != null && val == 0) { - return true; + // Starting from C++11, the value is required to be a literal. This means pre-C++11 parsing will not be correct here. + // But we don't currently have a way to check the C++ version here and in semantics code in general. + if (basicType.getKind() == Kind.eInt && basicType.isFromLiteral()) { + Long val = basicType.getAssociatedNumericalValue(); + if (val != null && val == 0) { + return true; + } } } return false; diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalBinary.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalBinary.java index 464e389ed49..8cfda662951 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalBinary.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalBinary.java @@ -151,6 +151,11 @@ public class EvalBinary extends CPPDependentEvaluation { } } } + + if (fType instanceof CPPBasicType) { + fType = ((CPPBasicType) fType).clone(~CPPBasicType.FROM_LITERAL); + } + return fType; } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalComma.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalComma.java index bc49e2cd3d0..aa8c24ce186 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalComma.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalComma.java @@ -28,6 +28,7 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPTemplateParameterMap; import org.eclipse.cdt.internal.core.dom.parser.DependentValue; import org.eclipse.cdt.internal.core.dom.parser.ITypeMarshalBuffer; import org.eclipse.cdt.internal.core.dom.parser.IntegralValue; +import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPBasicType; import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPEvaluation; import org.eclipse.cdt.internal.core.dom.parser.cpp.InstantiationContext; import org.eclipse.core.runtime.CoreException; @@ -145,6 +146,9 @@ public class EvalComma extends CPPDependentEvaluation { public IType getType() { if (fType == null) { fType = computeType(); + if (fType instanceof CPPBasicType) { + fType = ((CPPBasicType) fType).clone(~CPPBasicType.FROM_LITERAL); + } } return fType; } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalConditional.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalConditional.java index 3af16b1d397..49cc3df4639 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalConditional.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/EvalConditional.java @@ -39,6 +39,7 @@ import org.eclipse.cdt.internal.core.dom.parser.ITypeMarshalBuffer; import org.eclipse.cdt.internal.core.dom.parser.IntegralValue; import org.eclipse.cdt.internal.core.dom.parser.ProblemType; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPArithmeticConversion; +import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPBasicType; import org.eclipse.cdt.internal.core.dom.parser.cpp.CPPReferenceType; import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPEvaluation; import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPUnknownType; @@ -180,6 +181,14 @@ public class EvalConditional extends CPPDependentEvaluation { if (fValueCategory != null) return; + evaluateInternal(); + + if (fType instanceof CPPBasicType) { + fType = ((CPPBasicType) fType).clone(~CPPBasicType.FROM_LITERAL); + } + } + + private void evaluateInternal() { fValueCategory = PRVALUE; final ICPPEvaluation positive = fPositive == null ? fCondition : fPositive; diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOM.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOM.java index b7153a85968..4cff50e8adb 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOM.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/pdom/PDOM.java @@ -308,10 +308,13 @@ public class PDOM extends PlatformObject implements IPDOM { * 217.0 - Added nodiscard class/struct information, bug 534420 * 218.0 - Added nodiscard enums information, bug 534420 * 219.0 - Fix enums nodiscard information in the index from 8 byte to 1 byte, bug 534420 + * + * CDT 10.4 development + * 220.0 - Changed marshalling of CPPBasicType to store new "from literal" flag, bug 573764 */ - private static final int MIN_SUPPORTED_VERSION = version(219, 0); - private static final int MAX_SUPPORTED_VERSION = version(219, Short.MAX_VALUE); - private static final int DEFAULT_VERSION = version(219, 0); + private static final int MIN_SUPPORTED_VERSION = version(220, 0); + private static final int MAX_SUPPORTED_VERSION = version(220, Short.MAX_VALUE); + private static final int DEFAULT_VERSION = version(220, 0); private static int version(int major, int minor) { return (major << 16) + minor;