如何正确重写ASSERT代码以在msvc中传递/analyze?

发布于 2024-08-28 18:08:21 字数 583 浏览 11 评论 0原文

Visual Studio 添加了 C/C++ 代码分析 (/analyze),以帮助识别错误代码。这是一个非常好的功能,但是当您处理旧项目时,您可能会被大量的警告所淹没。

大多数问题的产生是因为旧代码在方法或函数的开头执行了一些 ASSERT。

我认为这是代码中使用的 ASSERT 定义(来自 afx.h

#define ASSERT(f)          DEBUG_ONLY((void) ((f) || !::AfxAssertFailedLine(THIS_FILE, __LINE__) || (AfxDebugBreak(), 0)))

示例代码:

ASSERT(pBytes != NULL);
*pBytes = 0; // <- warning C6011: Dereferencing NULL pointer 'pBytes'

我正在寻找一种简单、干净且安全的解决方案来解决这些警告,但这些警告并不意味着禁用这些警告。我是否提到过当前代码库中有很多这样的情况?

Visual Studio added code analysis (/analyze) for C/C++ in order to help identify bad code. This is quite a nice feature but when you deal with and old project you may be overwhelmed by the number of warnings.

Most of the problems are generating because the old code is doing some ASSERT at the beginning of the method or function.

I think this is the ASSERT definition used in the code (from afx.h)

#define ASSERT(f)          DEBUG_ONLY((void) ((f) || !::AfxAssertFailedLine(THIS_FILE, __LINE__) || (AfxDebugBreak(), 0)))

Example code:

ASSERT(pBytes != NULL);
*pBytes = 0; // <- warning C6011: Dereferencing NULL pointer 'pBytes'

I'm looking for an easy, clean and safe solution to solve these warnings that does not imply disabling these warnings. Did I mention that there are lots of occurrences in current codebase?

如果你对这篇内容有疑问,欢迎到本站社区发帖提问 参与讨论,获取更多帮助,或者扫码二维码加入 Web 技术交流群。

扫码二维码加入Web技术交流群

发布评论

需要 登录 才能够评论, 你可以免费 注册 一个本站的账号。

评论(6

凉栀 2024-09-04 18:08:21

/analyze 不保证产生相关且正确的警告。
它可能并且将会错过很多问题,并且还会给出许多误报(它识别为警告的事情,但这些事情是完全安全的并且永远不会实际发生)

期望 /analyze 出现零警告是不现实的。

它指出了一种情况,即您取消引用无法验证的指针是否始终有效。据 PREfast 所知,不能保证它永远不会为 NULL。

但这并不意味着它可以为NULL。只是证明其安全性所需的分析对于 PREfast 来说太复杂了。

您也许可以使用 Microsoft 特定的扩展 < code>__assume 告诉编译器它不应该产生此警告,但更好的解决方案是保留警告。每次使用 /analyze 进行编译时(不必每次编译时都如此),您应该验证它所出现的警告是否仍然是误报。

如果您正确使用断言(在编程期间捕获逻辑错误,防止不可能发生的情况),那么我认为您的代码没有问题,或者留下警告。添加代码来处理以下问题: 永远不会发生是毫无意义的,你无缘无故地添加了更多的代码和更多的复杂性(如果它永远不会发生,那么你就无法从中恢复,因为你完全不知道发生了什么。程序将处于的状态。您所知道的是它已经进入了您认为不可能的代码路径。

但是,如果您使用断言作为实际的错误处理(在特殊情况下,值可以为NULL,你只是希望它不会发生),那么它就是你的代码中的一个缺陷,那么就需要正确的错误处理(通常是异常)

。当 /analyze 向您发出警告时,请查看它们,如果是误报,请忽略它(不要压制它,因为虽然它是一个误报)。今天出现误报,明天您签入的代码可能会将其变成真正的问题)。

/analyze is not guaranteed to yield relevant and correct warnings.
It can and will miss a lot of issues, and it also gives a number of false positives (things it identifies as warnings, but which are perfectly safe and will never actually occur)

It is unrealistic to expect to have zero warnings with /analyze.

It has pointed out a situation where you dereference a pointer which it can not verify is always valid. As far as PREfast can tell, there's no guarantee that it will never be NULL.

But that doesn't mean it can be NULL. Just that the analysis required to prove that it's safe is too complex for PREfast.

You may be able to use the Microsoft-specific extension __assume to tell the compiler that it shouldn't produce this warning, but a better solution is to leave the warning. Every time you compile with /analyze (which need not be every time you compile), you should verify that the warnings it does come up with are still false positives.

If you use your asserts correctly (to catch logic error during programming, guarding against situations that cannot happen, then I see no problem with your code, or with leaving the warning. Adding code to handle a problem that can never occur is just pointless. You're adding more code and more complexity for no reason (if it can never occur, then you have no way of recovering from it, because you have absolutely no clue what state the program will be in. All you know is that it has entered a code path you thought impossible.

However, if you use your assert as actual error handling (the value can be NULL in exceptional cases, you just expect that it won't happen), then it is a defect in your code. Then proper error handling (exceptions, typically) is needed.

Never ever use asserts for problems that are possible. Use them to verify that the impossible isn't happening. And when /analyze gives you warnings, look at them. If it is a false positive, ignore it (don't suppress it, because while it's a false positive today, the code you check in tomorrow may turn it into a real issue).

叹倦 2024-09-04 18:08:21

PREFast 告诉您代码中存在缺陷;不要忽视它。事实上你确实有一个,但你只是匆匆地承认了这一点。问题是这样的:仅仅因为 pBytes 在开发过程中从未为 NULL测试并不意味着它不会投入生产。你不会处理这种可能性。 PREfast 知道这一点,并试图警告您,生产环境是充满敌意的,并且会让您的代码变成一堆冒烟、残缺不全、毫无价值的字节。

/rant

有两种方法可以解决这个问题:正确的方法和黑客。

正确的方法是在运行时处理 NULL 指针:

void DoIt(char* pBytes)
{
    assert(pBytes != NULL);
    if( !pBytes )
        return;
    *pBytes = 0;
}

这将使 PREfast 静音。

破解方法是使用注释。例如:

void DoIt(char* pBytes)
{
    assert(pBytes != NULL);
    __analysis_assume( pBytes );
    *pBytes = 0;
}

编辑:这是一个链接 描述 PREfast 注释。无论如何,这是一个起点。

PREFast is telling you that you have a defect in your code; don't ignore it. You do in fact have one, but you have only skittered around acknowleging it. The problem is this: just because pBytes has never been NULL in development & testing doesn't mean it won't be in production. You don't handle that eventuality. PREfast knows this, and is trying to warn you that production environments are hostile, and will leave your code a smoking, mutilated mass of worthless bytes.

/rant

There are two ways to fix this: the Right Way, and a hack.

The right way is to handle NULL pointers at runtime:

void DoIt(char* pBytes)
{
    assert(pBytes != NULL);
    if( !pBytes )
        return;
    *pBytes = 0;
}

This will silence PREfast.

The hack is to use an annotation. For example:

void DoIt(char* pBytes)
{
    assert(pBytes != NULL);
    __analysis_assume( pBytes );
    *pBytes = 0;
}

EDIT: Here's a link describing PREfast annotations. A starting point, anyway.

倾`听者〃 2024-09-04 18:08:21

首先,您的断言语句必须保证抛出或终止应用程序。经过一些实验,我发现在这种情况下 /analysis 会忽略模板函数、内联函数或普通函数中的所有实现。您必须改为使用宏和 do{}while(0) 技巧,并内联抑制

如果您查看 ATLENSURE() Microsoft 在他们的宏中使用 __analysis_assume(),他们还有几段非常好的文档说明为什么以及如何将 ATL 迁移到使用这个宏。

作为一个例子,我以相同的方式修改了 CPPUNIT_ASSERT 宏,以清除单元测试中的数千个警告。

#define CPPUNIT_ASSERT(condition)                                                                    \
do {    ( CPPUNIT_NS::Asserter::failIf( !(condition),                                                \
                                  CPPUNIT_NS::Message( "assertion failed" ),                         \
                                  CPPUNIT_SOURCELINE() ) ); __analysis_assume(!!(condition));        \
                                  __pragma( warning( push))                                          \
                                  __pragma( warning( disable: 4127 ))                                \
                                  } while(0)                                                         \
                                  __pragma( warning( pop))

Firstly your assertion statement must guarantee to throw or terminate the application. After some experimentation I found in this case /analyse ignores all implementation in either template functions, inline functions or normal functions. You must instead use macros and the do{}while(0) trick, with inline suppression of

If you look at the definition of ATLENSURE() Microsoft use __analyse_assume() in their macro, they also have several paragraphs of very good documentation on why and how they are migrating ATL to use this macro.

As an example of this I have modified the CPPUNIT_ASSERT macros in the same way to clean up thousands of warnings in our unit tests.

#define CPPUNIT_ASSERT(condition)                                                                    \
do {    ( CPPUNIT_NS::Asserter::failIf( !(condition),                                                \
                                  CPPUNIT_NS::Message( "assertion failed" ),                         \
                                  CPPUNIT_SOURCELINE() ) ); __analysis_assume(!!(condition));        \
                                  __pragma( warning( push))                                          \
                                  __pragma( warning( disable: 4127 ))                                \
                                  } while(0)                                                         \
                                  __pragma( warning( pop))
风流物 2024-09-04 18:08:21

请记住,ASSERT() 在零售版本中消失,因此上面的代码中的 C6011 警告绝对正确:您必须检查 pBytes 是否为非空以及执行 ASSERT()。如果调试错误中满足该条件,则 ASSERT() 只会将您的应用程序放入调试器中。

我在 /analyze 和 PREfast 上做了很多工作,所以如果您有其他问题,请告诉我。

remember, ASSERT() goes away in a retail build so the C6011 warning is absolutely correct in your code above: you must check that pBytes is non-null as well as doing the ASSERT(). the ASSERT() simply throws your app into a debugger if that condition is met in a debug bug.

I work a great deal on /analyze and PREfast, so if you have other questions, please feel to let me know.

ま柒月 2024-09-04 18:08:21

您似乎认为 ASSERT(ptr) 不知何故意味着 ptr 之后不为 NULL。这不是真的,代码分析器不会做出这样的假设。

You seem to assume that ASSERT(ptr) somehow means that ptr is not NULL afterwards. That's not true, and the code analyzer doesn't make that assumption.

_蜘蛛 2024-09-04 18:08:21

我的合著者 David LeBlanc 会告诉我这段代码无论如何都被破坏了,假设您使用的是 C++,您应该使用引用而不是指针,并且 ref 不能为 NULL :)

My co-author David LeBlanc would tell me this code is broken anyway, assuming you're using C++, you should use a reference rather than a pointer, and a ref can't be NULL :)

~没有更多了~
我们使用 Cookies 和其他技术来定制您的体验包括您的登录状态等。通过阅读我们的 隐私政策 了解更多相关信息。 单击 接受 或继续使用网站,即表示您同意使用 Cookies 和您的相关数据。
原文