Skip to content

Commit 58fd6c5

Browse files
xdoardoxdoardo
committed
Refactor syntactic and semantic checks of permissions strings out of the attribute handlers
1 parent 0a4e97e commit 58fd6c5

File tree

2 files changed

+118
-92
lines changed

2 files changed

+118
-92
lines changed

‎clang/lib/Sema/SemaDeclAttr.cpp‎

Lines changed: 38 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -2119,55 +2119,33 @@ static void handleCHERIOTMMIODevice(Sema &S, Decl *D, const ParsedAttr &Attr,
21192119
&PermissionsLiteralLoc);
21202120
}
21212121

2122-
if (Permissions.empty()) {
2123-
return D->addAttr(::new (S.Context) CHERIOTMMIODeviceAttr(
2124-
S.Context, Attr, DeviceName,
2125-
llvm::cheri::GlobalCapabilityImportAttr::defaultPermissions()));
2126-
}
2127-
2128-
auto ScratchPermissions = Permissions.str();
2129-
const auto *ValidPermissionSymbols =
2130-
llvm::cheri::GlobalCapabilityImportAttr::getValidPermissionSymbols();
2131-
2132-
for (char Symbol : *ValidPermissionSymbols) {
2133-
2134-
int SymbolOccurrences = 0;
2135-
auto new_end = std::remove_if(ScratchPermissions.begin(),
2136-
ScratchPermissions.end(), [&](char c) {
2137-
if (c == Symbol) {
2138-
++SymbolOccurrences;
2139-
return true;
2140-
}
2141-
return false;
2142-
});
2143-
2144-
ScratchPermissions.erase(new_end, ScratchPermissions.end());
2122+
std::string OwnedPermissions = Permissions.str();
21452123

2146-
if (SymbolOccurrences > 1) {
2147-
S.Diag(Attr.getLoc(), diag::warn_cheriot_cap_permissions_duplicate_sym)
2148-
<< Attr.getAttrName() << "'" + std::string({Symbol}) + "'"
2149-
<< Permissions;
2150-
}
2151-
}
2124+
auto DuplicateSymbolCallback = [&Attr, &S,
2125+
Permissions](char DuplicateSymbol) {
2126+
S.Diag(Attr.getLoc(), diag::warn_cheriot_cap_permissions_duplicate_sym)
2127+
<< Attr.getAttrName() << "'" + std::string({DuplicateSymbol}) + "'"
2128+
<< Permissions;
2129+
};
21522130

2153-
if (!ScratchPermissions.empty()) {
2131+
auto ExtraneousSymbolCallback = [&Attr, &S,
2132+
Permissions](StringRef ExtraneousSymbols) {
21542133
S.Diag(Attr.getLoc(),
21552134
diag::err_cheriot_malformed_cap_permissions_unknown_sym)
2156-
<< Attr.getAttrName() << ScratchPermissions << Permissions;
2157-
return;
2158-
}
2159-
2160-
std::string OwnedPermissions = Permissions.str();
2161-
auto ConstraintsCheck =
2162-
llvm::cheri::GlobalCapabilityImportAttr::semaCheckPermissions(
2163-
OwnedPermissions);
2135+
<< Attr.getAttrName() << ExtraneousSymbols << Permissions;
2136+
};
21642137

2165-
if (ConstraintsCheck.has_value()) {
2138+
auto FailedSemanticCheckCallback = [&Attr, &S,
2139+
Permissions](StringRef Reason) {
21662140
S.Diag(Attr.getLoc(),
21672141
diag::err_cheriot_malformed_cap_permissions_invalid_dep)
2168-
<< Attr.getAttrName() << ConstraintsCheck.value() << OwnedPermissions;
2142+
<< Attr.getAttrName() << Reason << Permissions;
2143+
};
2144+
2145+
if (!llvm::cheri::GlobalCapabilityImportAttr::checkPermissions(
2146+
OwnedPermissions, DuplicateSymbolCallback, ExtraneousSymbolCallback,
2147+
FailedSemanticCheckCallback))
21692148
return;
2170-
}
21712149

21722150
return D->addAttr(::new (S.Context) CHERIOTMMIODeviceAttr(
21732151
S.Context, Attr, DeviceName, OwnedPermissions));
@@ -2188,55 +2166,33 @@ static void handleCHERIOTSharedObject(Sema &S, Decl *D, const ParsedAttr &Attr,
21882166
&PermissionsLiteralLoc);
21892167
}
21902168

2191-
if (Permissions.empty()) {
2192-
return D->addAttr(::new (S.Context) CHERIOTSharedObjectAttr(
2193-
S.Context, Attr, ObjectName,
2194-
llvm::cheri::GlobalCapabilityImportAttr::defaultPermissions()));
2195-
}
2196-
2197-
auto ScratchPermissions = Permissions.str();
2198-
const auto *ValidPermissionSymbols =
2199-
llvm::cheri::GlobalCapabilityImportAttr::getValidPermissionSymbols();
2200-
2201-
for (char Symbol : *ValidPermissionSymbols) {
2202-
2203-
int SymbolOccurrences = 0;
2204-
auto new_end = std::remove_if(ScratchPermissions.begin(),
2205-
ScratchPermissions.end(), [&](char c) {
2206-
if (c == Symbol) {
2207-
++SymbolOccurrences;
2208-
return true;
2209-
}
2210-
return false;
2211-
});
2212-
2213-
ScratchPermissions.erase(new_end, ScratchPermissions.end());
2169+
std::string OwnedPermissions = Permissions.str();
22142170

2215-
if (SymbolOccurrences > 1) {
2216-
S.Diag(Attr.getLoc(), diag::warn_cheriot_cap_permissions_duplicate_sym)
2217-
<< Attr.getAttrName() << "'" + std::string({Symbol}) + "'"
2218-
<< Permissions;
2219-
}
2220-
}
2171+
auto DuplicateSymbolCallback = [&Attr, &S,
2172+
Permissions](char DuplicateSymbol) {
2173+
S.Diag(Attr.getLoc(), diag::warn_cheriot_cap_permissions_duplicate_sym)
2174+
<< Attr.getAttrName() << "'" + std::string({DuplicateSymbol}) + "'"
2175+
<< Permissions;
2176+
};
22212177

2222-
if (!ScratchPermissions.empty()) {
2178+
auto ExtraneousSymbolCallback = [&Attr, &S,
2179+
Permissions](StringRef ExtraneousSymbols) {
22232180
S.Diag(Attr.getLoc(),
22242181
diag::err_cheriot_malformed_cap_permissions_unknown_sym)
2225-
<< Attr.getAttrName() << ScratchPermissions << Permissions;
2226-
return;
2227-
}
2228-
2229-
std::string OwnedPermissions = Permissions.str();
2230-
auto ConstraintsCheck =
2231-
llvm::cheri::GlobalCapabilityImportAttr::semaCheckPermissions(
2232-
OwnedPermissions);
2182+
<< Attr.getAttrName() << ExtraneousSymbols << Permissions;
2183+
};
22332184

2234-
if (ConstraintsCheck.has_value()) {
2185+
auto FailedSemanticCheckCallback = [&Attr, &S,
2186+
Permissions](StringRef Reason) {
22352187
S.Diag(Attr.getLoc(),
22362188
diag::err_cheriot_malformed_cap_permissions_invalid_dep)
2237-
<< Attr.getAttrName() << ConstraintsCheck.value() << Permissions;
2189+
<< Attr.getAttrName() << Reason << Permissions;
2190+
};
2191+
2192+
if (!llvm::cheri::GlobalCapabilityImportAttr::checkPermissions(
2193+
OwnedPermissions, DuplicateSymbolCallback, ExtraneousSymbolCallback,
2194+
FailedSemanticCheckCallback))
22382195
return;
2239-
}
22402196

22412197
D->addAttr(::new (S.Context) CHERIOTSharedObjectAttr(
22422198
S.Context, Attr, ObjectName, OwnedPermissions));

‎llvm/include/llvm/IR/Cheri.h‎

Lines changed: 80 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "llvm/IR/Instruction.h"
2323
#include "llvm/IR/Module.h"
2424
#include "llvm/IR/Type.h"
25+
#include <algorithm>
2526

2627
namespace llvm {
2728

@@ -181,8 +182,9 @@ class GlobalCapabilityImportAttr {
181182
///
182183
/// If successful, this function modifies in-place the given string ref to a
183184
/// known format.
184-
static std::optional<std::string>
185-
semaCheckPermissions(std::string &Permissions) {
185+
static bool semanticCheckPermissions(
186+
std::string &Permissions,
187+
std::function<void(std::string)> FailedSemanticCheckCallback) {
186188

187189
auto PermissionsRef = StringRef(Permissions);
188190
auto HasRead = PermissionsRef.contains(ReadPermissionSymbol);
@@ -191,23 +193,91 @@ class GlobalCapabilityImportAttr {
191193
auto HasCap = PermissionsRef.contains(CapPermissionSymbol);
192194

193195
if (!HasRead && !HasWrite) {
194-
return std::optional("does not contain either read (" +
195-
std::string(ReadPermissionSymbol) + ") or write (" +
196-
std::string(WritePermissionSymbol) + ")");
196+
FailedSemanticCheckCallback(
197+
"does not contain either read (" + std::string(ReadPermissionSymbol) +
198+
") or write (" + std::string(WritePermissionSymbol) + ")");
199+
return false;
197200
}
198201

199202
if (HasMut && (!HasRead || !HasCap)) {
200-
return std::optional("contains mut (" + std::string(MutPermissionSymbol) +
201-
") but does not have both read (" +
202-
std::string(ReadPermissionSymbol) + ") and cap (" +
203-
std::string(CapPermissionSymbol) + ")");
203+
FailedSemanticCheckCallback(
204+
"contains mut (" + std::string(MutPermissionSymbol) +
205+
") but does not have both read (" +
206+
std::string(ReadPermissionSymbol) + ") and cap (" +
207+
std::string(CapPermissionSymbol) + ")");
208+
return false;
204209
}
205210

206211
Permissions = (HasRead ? std::string(ReadPermissionSymbol) : "") +
207212
(HasWrite ? std::string(WritePermissionSymbol) : "") +
208213
(HasCap ? std::string(CapPermissionSymbol) : "") +
209214
(HasMut ? std::string(MutPermissionSymbol) : "");
210-
return std::nullopt;
215+
return true;
216+
}
217+
218+
/// Checks whether a permission encoding respects the syntactic constraints.
219+
static bool syntaxCheckPermissions(
220+
std::string &Permissions,
221+
std::function<void(char)> DuplicateSymbolCallback,
222+
std::function<void(StringRef)> ExtraneousSymbolCallback) {
223+
224+
const auto ValidSymbolsSize = ValidSymbols.size();
225+
std::vector<bool> SymbolsCounter(ValidSymbolsSize, false);
226+
std::string ExtraneousSymbols("");
227+
const auto *ValidSymbolsBegin = std::begin(ValidSymbols);
228+
bool Duplicate;
229+
230+
for (char C : Permissions) {
231+
Duplicate = false;
232+
233+
const unsigned long Pos =
234+
std::find(ValidSymbolsBegin, ValidSymbolsBegin + ValidSymbolsSize,
235+
C) -
236+
ValidSymbolsBegin;
237+
238+
if (Pos < ValidSymbolsSize) {
239+
if (SymbolsCounter[Pos]) {
240+
Duplicate = true;
241+
}
242+
243+
SymbolsCounter[Pos] = true;
244+
} else {
245+
ExtraneousSymbols.push_back(C);
246+
}
247+
248+
if (Duplicate)
249+
DuplicateSymbolCallback(C);
250+
}
251+
252+
if (!ExtraneousSymbols.empty()) {
253+
ExtraneousSymbolCallback(ExtraneousSymbols);
254+
return false;
255+
}
256+
257+
return true;
258+
}
259+
260+
static bool checkPermissions(
261+
std::string &Permissions,
262+
std::function<void(char)> DuplicateSymbolCallback = [](char) {},
263+
std::function<void(StringRef)> ExtraneousSymbolCallback =
264+
[](StringRef) {},
265+
std::function<void(std::string)> FailedSemanticCheckCallback =
266+
[](std::string) {}) {
267+
268+
// If no permissions were set, use the default ones.
269+
if (Permissions.empty()) {
270+
Permissions = defaultPermissions();
271+
}
272+
273+
// Perform the syntactic checks.
274+
if (!syntaxCheckPermissions(Permissions, DuplicateSymbolCallback,
275+
ExtraneousSymbolCallback)) {
276+
return false;
277+
}
278+
279+
// Semantics checks.
280+
return semanticCheckPermissions(Permissions, FailedSemanticCheckCallback);
211281
}
212282

213283
/// Returns the default permissions.

0 commit comments

Comments
 (0)