[SR-14941] Introduce compiler warning for leading-zero octal notation

[swift-ci]

Previous ID	SR-14941
Radar	rdar://problem/22123688
Original Reporter	eaigner (JIRA User)
Type	Improvement

Additional Detail from JIRA

Votes	0
Component/s
Labels	Improvement, StarterBug
Assignee	None
Priority	Medium

md5: 639a9bd96ac9c3918c5433098fa9ad29

Issue Description:

I had a bug in my code for quite some time that went by unnoticed, because I didn't realize Swift handled octal numbers differently. Specifically I was making the following call

mkfifo(path, 0644)

This should obviously read `0o644`, but went by unnoticed, since it just resulted in an elevated user permission.

It would be great if the compiler could warn in such instances where numbers are typed with leading zeroes, maybe something of the form "Did you mean to write an octal number?"

[typesanitizer]

I agree, this is a good idea.

There is one edge case though, if someone is using a leading zero for consistency reasons, say in a literal like [0055, 0550, 5500, 0505, 5005, 5050, 5500], then this warning will give a bunch of false positives, which would be unfortunate...

@swift-ci create

[swift-ci]

Comment by Erik Aigner (JIRA)

I would say a potential security issue is more important then using 0 as padding. If someone wanted to line up a number matrix (or I don't know what your edge case would be), he could as well substitute a space for the leading zero.

[swift-ci]

Comment by Erik Aigner (JIRA)

In any case, if leading zeroes are wanted, there is always the option to disable certain compiler warnings.

[typesanitizer]

I would say a potential security issue is more important then using 0 as padding.

I agree, that's why I said this is a good idea overall. It's just that we need to provide a way to suppress the warning if it is superfluous.

there is always the option to disable certain compiler warnings

That's not the case in general. Certain warnings have ad-hoc ways of being suppressed, but most warnings cannot be disabled on an individual basis.

For this particular one, we'd probably need to come up with a way to suppress it locally if we were to add it.

[typesanitizer]

I think the right place to do this would in the implementation of CSApply.cpp's {{handleIntegerLiteralExpr}} function.

Under the following circumstances:

• The literal has a leading zero not following by an 'o'.

• All digits are in the range 0-7.

• The expression is an argument to a function which was imported from C where the corresponding argument type is 'mode_t'.

In this situation, emit a warning with a fix-it to add an 'o' after the 0.

[idrougge]

I think this is quite undesirable. Leading zeros are very useful for auto-generated or tabulated literals — the fact that they have another meaning in another language (whose example Swift has chosen not to follow) is irrelevant.

Generating a warning for no specific reason makes leading zeros basically useless.

[swift-ci]

Comment by Erik Aigner (JIRA)

What exactly prevents you from putting a leading space instead of a leading zero? Where is it written that tabulated data has to have leading zeroes?

[swift-ci]

Comment by Erik Aigner (JIRA)

Also, since swift chose to support _ for number readability, you might as well allow _ as a prefix instead of abusing 0.

[typesanitizer]

@idrougge, that is why I proposed a very narrow warning, not for arbitrary situations involving leading zeroes. Swift needs to talk to C APIs; in fact, Swift has a special ClangImporter sub-system to make importing APIs from C-based languages easy.

Dismissing someone else's concern as "irrelevant" is not particularly helpful. In this bug report, there is a very concrete use case that has been given, which can lead to a security issue. I don't think anyone is arguing that all leading zeroes should lead to warnings. Adding a targeted warning should be doable without issuing additional warnings for the vast majority of Swift code using leading zeroes.

[xwu]

In the same vein, though, dismissing the legitimate use of leading zeros as “abuse,” when it is not only conventional mathematic notation but so commonly a source of error in C-family programming languages that it is the reason for Swift to deviate from C octal notation, is not appropriate.

I find the maxim that everything permitted in a programming language will be used by somebody to hold true generally, and given that leading zeros have been explicitly contemplated since at least the very first public release of Swift to mean what it does today, we should be very careful about how to proceed here. If somehow a warning can be very narrowly tailored (say, only when interfacing with C APIs that take arguments of type mode_t, just as Varun argues, or creating some sort of annotation for APIs that typically take octal arguments), then that seems plenty wise.

But the use of leading zeros is most certainly not an “abuse” (nor “edge case,” given that it’s the crux of the rationale for Swift’s decision to deviate from prior art), and warning about all such uses (which is, by my read, what Erik here is arguing for) in my view is a nonstarter.

Overall, I think there’s sufficient design finesse and subject matter expertise required here for an appropriate solution that I wouldn’t characterize it as a starter bug.

[swift-ci]

Comment by Erik Aigner (JIRA)

I am not arguing for anything except a warning for octal permission flags for C APIs. I couldn't care less about leading zeroes, as nonsensical they might be.

[xwu]

Sure, I think a warning for that scenario is pretty reasonable. Again, though, let’s not disparage other people’s coding preferences as “nonsensical.” It doesn’t advance your point and comes across as antagonistic.